diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp index 30047c9505b1..f295545cde9b 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.cpp @@ -1290,6 +1290,54 @@ std::array, 3> field_t::slice(const uint8_t msb, const return result; } +template +std::pair, field_t> field_t::split_at(const size_t lsb_index, + const size_t num_bits) const +{ + ASSERT(lsb_index < num_bits); + ASSERT(num_bits <= grumpkin::MAX_NO_WRAP_INTEGER_BIT_LENGTH); + + const uint256_t value = get_value(); + const uint256_t hi = value >> lsb_index; + const uint256_t lo = value % (uint256_t(1) << lsb_index); + + if (is_constant()) { + // If `*this` is constant, we can return the split values directly + ASSERT(lo + (hi << lsb_index) == value); + return std::make_pair(field_t(lo), field_t(hi)); + } + + // Handle edge case when lsb_index == 0 + if (lsb_index == 0) { + ASSERT(hi == value); + ASSERT(lo == 0); + create_range_constraint(num_bits, "split_at: hi value too large."); + return std::make_pair(field_t(0), *this); + } + + Builder* ctx = get_context(); + ASSERT(ctx != nullptr); + + field_t lo_wit(witness_t(ctx, lo)); + field_t hi_wit(witness_t(ctx, hi)); + + // Ensure that `lo_wit` is in the range [0, 2^lsb_index - 1] + lo_wit.create_range_constraint(lsb_index, "split_at: lo value too large."); + + // Ensure that `hi_wit` is in the range [0, 2^(num_bits - lsb_index) - 1] + hi_wit.create_range_constraint(num_bits - lsb_index, "split_at: hi value too large."); + + // Check that *this = lo_wit + hi_wit * 2^{lsb_index} + const field_t reconstructed = lo_wit + (hi_wit * field_t(uint256_t(1) << lsb_index)); + assert_equal(reconstructed, "split_at: decomposition failed"); + + // Set the origin tag for both witnesses + lo_wit.set_origin_tag(tag); + hi_wit.set_origin_tag(tag); + + return std::make_pair(lo_wit, hi_wit); +} + /** * @brief Build constraints establishing the decomposition of `*this` into bits. * diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.hpp index 295dfe6997cc..9d64a4ee228d 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.hpp @@ -339,6 +339,9 @@ template class field_t { std::array slice(uint8_t msb, uint8_t lsb) const; + std::pair, field_t> split_at( + const size_t lsb_index, const size_t num_bits = grumpkin::MAX_NO_WRAP_INTEGER_BIT_LENGTH) const; + bool_t is_zero() const; void create_range_constraint(size_t num_bits, std::string const& msg = "field_t::range_constraint") const; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.test.cpp index 4ab429965782..1605ffb21971 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/field/field.test.cpp @@ -4,6 +4,7 @@ #include "barretenberg/circuit_checker/circuit_checker.hpp" #include "barretenberg/common/streams.hpp" #include "barretenberg/numeric/random/engine.hpp" +#include "barretenberg/numeric/uint256/uint256.hpp" #include "barretenberg/stdlib/primitives/circuit_builders/circuit_builders.hpp" #include #include @@ -859,6 +860,56 @@ template class stdlib_field : public testing::Test { EXPECT_TRUE(builder.err() == "slice: hi value too large."); } + static void test_split_at() + { + Builder builder = Builder(); + + // Test different bit sizes + std::vector test_bit_sizes = { 8, 16, 32, 100, 252 }; + + // Lambda to check split_at functionality + auto check_split_at = [&](const field_ct& a, size_t start, size_t num_bits) { + const uint256_t a_native = a.get_value(); + auto split_data = a.split_at(start, num_bits); + EXPECT_EQ(split_data.first.get_value(), a_native & ((uint256_t(1) << start) - 1)); + EXPECT_EQ(split_data.second.get_value(), (a_native >> start) & ((uint256_t(1) << num_bits) - 1)); + + if (a.is_constant()) { + EXPECT_TRUE(split_data.first.is_constant()); + EXPECT_TRUE(split_data.second.is_constant()); + } + + if (start == 0) { + EXPECT_TRUE(split_data.first.is_constant()); + EXPECT_TRUE(split_data.first.get_value() == 0); + EXPECT_EQ(split_data.second.get_value(), a.get_value()); + } + }; + + for (size_t num_bits : test_bit_sizes) { + uint256_t a_native = engine.get_random_uint256() & ((uint256_t(1) << num_bits) - 1); + + // check split_at for a constant + field_ct a_constant(a_native); + check_split_at(a_constant, 0, num_bits); + check_split_at(a_constant, num_bits / 4, num_bits); + check_split_at(a_constant, num_bits / 3, num_bits); + check_split_at(a_constant, num_bits / 2, num_bits); + check_split_at(a_constant, num_bits - 1, num_bits); + + // check split_at for a witness + field_ct a_witness(witness_ct(&builder, a_native)); + check_split_at(a_witness, 0, num_bits); + check_split_at(a_witness, num_bits / 4, num_bits); + check_split_at(a_witness, num_bits / 3, num_bits); + check_split_at(a_witness, num_bits / 2, num_bits); + check_split_at(a_witness, num_bits - 1, num_bits); + } + + bool result = CircuitChecker::check(builder); + EXPECT_EQ(result, true); + } + static void test_three_bit_table() { Builder builder = Builder(); @@ -1258,7 +1309,10 @@ template class stdlib_field : public testing::Test { static void test_origin_tag_consistency() { Builder builder = Builder(); - auto a = field_ct(witness_ct(&builder, bb::fr::random_element())); + // Randomly generate a and b (a must ≤ 252 bits) + uint256_t a_val = + uint256_t(bb::fr::random_element()) & ((uint256_t(1) << grumpkin::MAX_NO_WRAP_INTEGER_BIT_LENGTH) - 1); + auto a = field_ct(witness_ct(&builder, a_val)); auto b = field_ct(witness_ct(&builder, bb::fr::random_element())); EXPECT_TRUE(a.get_origin_tag().is_empty()); EXPECT_TRUE(b.get_origin_tag().is_empty()); @@ -1362,6 +1416,12 @@ template class stdlib_field : public testing::Test { EXPECT_EQ(element.get_origin_tag(), submitted_value_origin_tag); } + // Split preserves tags + const size_t num_bits = uint256_t(a.get_value()).get_msb() + 1; + auto split_data = a.split_at(num_bits / 2, num_bits); + EXPECT_EQ(split_data.first.get_origin_tag(), submitted_value_origin_tag); + EXPECT_EQ(split_data.second.get_origin_tag(), submitted_value_origin_tag); + // Decomposition preserves tags auto decomposed_bits = a.decompose_into_bits(); @@ -1555,6 +1615,10 @@ TYPED_TEST(stdlib_field, test_slice_random) { TestFixture::test_slice_random(); } +TYPED_TEST(stdlib_field, test_split_at) +{ + TestFixture::test_split_at(); +} TYPED_TEST(stdlib_field, test_three_bit_table) { TestFixture::test_three_bit_table(); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/arithmetic.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/arithmetic.cpp index dcc0329b65d8..0539ac7c9c07 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/arithmetic.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/arithmetic.cpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: not started, auditors: [], date: YYYY-MM-DD } +// internal: { status: done, auditors: [suyash], date: 2025-07-23 } // external_1: { status: not started, auditors: [], date: YYYY-MM-DD } // external_2: { status: not started, auditors: [], date: YYYY-MM-DD } // ===================== @@ -18,6 +18,7 @@ uint uint::operator+(const uint& other) const ASSERT(context == other.context || (context != nullptr && other.context == nullptr) || (context == nullptr && other.context != nullptr)); + Builder* ctx = (context == nullptr) ? other.context : context; if (is_constant() && other.is_constant()) { @@ -25,6 +26,13 @@ uint uint::operator+(const uint& other) const } // N.B. We assume that additive_constant is nonzero ONLY if value is constant + if (!is_constant()) { + BB_ASSERT_EQ(additive_constant, 0ULL, "uint::operator+ expected additive_constant to be zero"); + } + if (!other.is_constant()) { + BB_ASSERT_EQ(other.additive_constant, 0ULL, "uint::operator+ expected other.additive_constant to be zero"); + } + const uint256_t lhs = get_value(); const uint256_t rhs = other.get_value(); const uint256_t constants = (additive_constant + other.additive_constant) & MASK; @@ -48,7 +56,7 @@ uint uint::operator+(const uint& other) const uint result(ctx); result.witness_index = gate.c; - result.witness_status = WitnessStatus::WEAK_NORMALIZED; + result.normalize(); return result; } @@ -67,12 +75,19 @@ uint uint::operator-(const uint& other) const } // N.B. We assume that additive_constant is nonzero ONLY if value is constant + if (!is_constant()) { + BB_ASSERT_EQ(additive_constant, 0ULL, "uint::operator- expected additive_constant to be zero"); + } + if (!other.is_constant()) { + BB_ASSERT_EQ(other.additive_constant, 0ULL, "uint::operator- expected other.additive_constant to be zero"); + } + const uint32_t lhs_idx = is_constant() ? ctx->zero_idx : witness_index; const uint32_t rhs_idx = other.is_constant() ? ctx->zero_idx : other.witness_index; const uint256_t lhs = get_value(); const uint256_t rhs = other.get_value(); - const uint256_t constant_term = (additive_constant - other.additive_constant); + const uint256_t constant_term = CIRCUIT_UINT_MAX_PLUS_ONE + (additive_constant - other.additive_constant); const uint256_t difference = CIRCUIT_UINT_MAX_PLUS_ONE + lhs - rhs; const uint256_t overflow = difference >> width; @@ -87,14 +102,14 @@ uint uint::operator-(const uint& other) const FF::neg_one(), FF::neg_one(), -FF(CIRCUIT_UINT_MAX_PLUS_ONE), - CIRCUIT_UINT_MAX_PLUS_ONE + constant_term, + constant_term, }; ctx->create_balanced_add_gate(gate); uint result(ctx); result.witness_index = gate.c; - result.witness_status = WitnessStatus::WEAK_NORMALIZED; + result.normalize(); return result; } @@ -102,6 +117,9 @@ uint uint::operator-(const uint& other) const template uint uint::operator*(const uint& other) const { + ASSERT(context == other.context || (context != nullptr && other.context == nullptr) || + (context == nullptr && other.context != nullptr)); + Builder* ctx = (context == nullptr) ? other.context : context; if (is_constant() && other.is_constant()) { @@ -111,6 +129,14 @@ uint uint::operator*(const uint& other) const return other * (*this); } + // N.B. We assume that additive_constant is nonzero ONLY if value is constant + if (!is_constant()) { + BB_ASSERT_EQ(additive_constant, 0ULL, "uint::operator* expected additive_constant to be zero"); + } + if (!other.is_constant()) { + BB_ASSERT_EQ(other.additive_constant, 0ULL, "uint::operator* expected other.additive_constant to be zero"); + } + const uint32_t rhs_idx = other.is_constant() ? ctx->zero_idx : other.witness_index; const uint256_t lhs = ctx->get_variable(witness_index); @@ -139,9 +165,8 @@ uint uint::operator*(const uint& other) const ctx->decompose_into_default_range(gate.d, width); uint result(ctx); - result.accumulators = constrain_accumulators(ctx, gate.c); result.witness_index = gate.c; - result.witness_status = WitnessStatus::OK; + result.normalize(); return result; } @@ -184,20 +209,17 @@ std::pair, uint> uint::d Builder* ctx = (context == nullptr) ? other.context : context; // We want to force the divisor to be non-zero, as this is an error state - if (other.is_constant() && other.get_value() == 0) { - // TODO: should have an actual error handler! - const uint32_t one = ctx->add_variable(FF::one()); - ctx->assert_equal_constant(one, FF::zero(), "plookup_arithmetic: divide by zero!"); - } else if (!other.is_constant()) { - const bool_t is_divisor_zero = field_t(other).is_zero(); - ctx->assert_equal_constant(is_divisor_zero.witness_index, FF::zero(), "plookup_arithmetic: divide by zero!"); - } + static_cast>(other).assert_is_not_zero("uint_arithmetic: divide by zero!"); + // If both are constants, we can compute the quotient and remainder directly if (is_constant() && other.is_constant()) { const uint remainder(ctx, additive_constant % other.additive_constant); const uint quotient(ctx, additive_constant / other.additive_constant); return std::make_pair(quotient, remainder); - } else if (witness_index == other.witness_index) { + } + + // If the divisor and dividend are the same witness, we can return a quotient of 1 and a remainder of 0. + if (witness_index == other.witness_index) { const uint remainder(context, 0); const uint quotient(context, 1); return std::make_pair(quotient, remainder); @@ -248,13 +270,11 @@ std::pair, uint> uint::d ctx->decompose_into_default_range(delta_idx, width); uint quotient(ctx); quotient.witness_index = quotient_idx; - quotient.accumulators = constrain_accumulators(ctx, quotient.witness_index); - quotient.witness_status = WitnessStatus::OK; + quotient.normalize(); uint remainder(ctx); remainder.witness_index = remainder_idx; - remainder.accumulators = constrain_accumulators(ctx, remainder.witness_index); - remainder.witness_status = WitnessStatus::OK; + remainder.normalize(); return std::make_pair(quotient, remainder); } diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/comparison.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/comparison.cpp index 294c93cb23a3..99fea541d128 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/comparison.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/comparison.cpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: not started, auditors: [], date: YYYY-MM-DD } +// internal: { status: done, auditors: [suyash], date: 2025-07-23 } // external_1: { status: not started, auditors: [], date: YYYY-MM-DD } // external_2: { status: not started, auditors: [], date: YYYY-MM-DD } // ===================== diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/logic.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/logic.cpp index 9d6ef527eb02..7f462949032b 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/logic.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/logic.cpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: not started, auditors: [], date: YYYY-MM-DD } +// internal: { status: done, auditors: [suyash], date: 2025-07-23 } // external_1: { status: not started, auditors: [], date: YYYY-MM-DD } // external_2: { status: not started, auditors: [], date: YYYY-MM-DD } // ===================== @@ -47,55 +47,64 @@ uint uint::operator>>(const size_t shift) cons return uint(context, (additive_constant >> shift) & MASK); } - if (witness_status != WitnessStatus::OK) { - normalize(); - } + // Normalize before shifting. + normalize(); if (shift == 0) { return *this; } - uint64_t bits_per_hi_limb; - // last limb will not likely bit `bits_per_limb`. Need to be careful with our range check - if (shift >= ((width / bits_per_limb) * bits_per_limb)) { - bits_per_hi_limb = width % bits_per_limb; - } else { - bits_per_hi_limb = bits_per_limb; - } - const uint64_t slice_bit_position = shift % bits_per_limb; + // Example for uint32_t: + // + // |<------ acc[2] ------>||<----------- acc[1] ----------->||<------- acc[0] ------->| + // val: [ 31 30 29 28 27 26 25 24 23 22 21 20 19 18 17 16 15 14 13 12 11 10 9 8 7 6 5 4 3 2 1 0 ] + // ↑ + // [<--------------- keep --------------->][<-------- discard -------->] + // + // out: [ 31 30 29 28 27 26 25 24 23 22 21 20 19 18 17 16 15 ] + // |<------ acc[2] ------>||<----- acc[1].hi ----->| + // + // Suppose the shift is 15, then we must discard the 15 least significant bits of the accumulator. + // The accumulator is split into three parts, so we clearly need to split acc[1]. On splitting, we must + // discard the lower slice of acc[1] and keep the upper slice. Thus, the updated uint value will be: + // + // acc[1].hi + (acc[2] << (24 - 15)) + // + // Let us first fetch the accumlator that needs to be sliced. const size_t accumulator_index = shift / bits_per_limb; - const uint32_t slice_index = accumulators[accumulator_index]; - const uint64_t slice_value = uint256_t(context->get_variable(slice_index)).data[0]; - - const uint64_t slice_lo = slice_value % (1ULL << slice_bit_position); - const uint64_t slice_hi = slice_value >> slice_bit_position; - const uint32_t slice_lo_idx = slice_bit_position ? context->add_variable(slice_lo) : context->zero_idx; - const uint32_t slice_hi_idx = - (slice_bit_position != bits_per_limb) ? context->add_variable(slice_hi) : context->zero_idx; - - context->create_big_add_gate( - { slice_index, slice_lo_idx, context->zero_idx, slice_hi_idx, -1, 1, 0, (1 << slice_bit_position), 0 }); - - if (slice_bit_position != 0) { - context->create_new_range_constraint(slice_lo_idx, (1ULL << slice_bit_position) - 1); - } - context->create_new_range_constraint(slice_hi_idx, (1ULL << (bits_per_hi_limb - slice_bit_position)) - 1); + const uint32_t slice_witness_index = accumulators[accumulator_index]; + const field_t acc_to_be_sliced = field_t::from_witness_index(context, slice_witness_index); + + // Now, lets calculate: + // (i) bit position (from lsb) at which we need to slice. + // (ii) number of bits in the slice based on whether it is the highest slice or not. + const size_t slice_bit_position = shift % bits_per_limb; + const bool is_slice_hi = (accumulator_index == num_accumulators() - 1); + const uint8_t num_bits_per_limb = is_slice_hi ? bits_in_high_limb : bits_per_limb; + + // Finally, we can slice the accumulator. + // The slice_hi will be the upper slice of the accumulator, which we will keep. + // The slice_lo will be the lower slice of the accumulator, which we will discard. + // Its important to note that although slice_lo is not used here, it is still created and properly constrained + // in the split_at function. + field_t slice_hi = acc_to_be_sliced.split_at(slice_bit_position, num_bits_per_limb).second; + + // Now we reconstruct the shifted uint value. std::vector> sublimbs; - sublimbs.emplace_back(field_t::from_witness_index(context, slice_hi_idx)); + sublimbs.emplace_back(slice_hi); const size_t start = accumulator_index + 1; field_t coefficient(context, uint64_t(1ULL << (start * bits_per_limb - shift))); field_t shifter(context, uint64_t(1ULL << bits_per_limb)); for (size_t i = accumulator_index + 1; i < num_accumulators(); ++i) { - sublimbs.emplace_back(field_t::from_witness_index(context, accumulators[i]) * - field_t(coefficient)); + sublimbs.emplace_back(field_t::from_witness_index(context, accumulators[i]) * coefficient); coefficient *= shifter; } uint32_t result_index = field_t::accumulate(sublimbs).get_witness_index(); uint result(context); result.witness_index = result_index; - result.witness_status = WitnessStatus::WEAK_NORMALIZED; + result.normalize(); return result; } @@ -109,70 +118,69 @@ uint uint::operator<<(const size_t shift) cons return uint(context, (additive_constant << shift) & MASK); } - if (witness_status != WitnessStatus::OK) { - normalize(); - } + // Normalize before shifting. + normalize(); if (shift == 0) { return *this; } - uint64_t slice_bit_position; - size_t accumulator_index; - size_t bits_per_hi_limb; - // most significant limb is only 2 bits long (for u32), need to be careful about which slice we index, - // and how large the range check is on our hi limb - if (shift < (width - ((width / bits_per_limb) * bits_per_limb))) { - bits_per_hi_limb = width % bits_per_limb; - slice_bit_position = bits_per_hi_limb - (shift % bits_per_hi_limb); - accumulator_index = num_accumulators() - 1; - } else { - const size_t offset = width % bits_per_limb; - slice_bit_position = bits_per_limb - ((shift - offset) % bits_per_limb); - accumulator_index = num_accumulators() - 2 - ((shift - offset) / bits_per_limb); - bits_per_hi_limb = bits_per_limb; - } - - const uint32_t slice_index = accumulators[accumulator_index]; - const uint64_t slice_value = uint256_t(context->get_variable(slice_index)).data[0]; - - const uint64_t slice_lo = slice_value % (1ULL << slice_bit_position); - const uint64_t slice_hi = slice_value >> slice_bit_position; - const uint32_t slice_lo_idx = slice_bit_position ? context->add_variable(slice_lo) : context->zero_idx; - const uint32_t slice_hi_idx = - (slice_bit_position != bits_per_hi_limb) ? context->add_variable(slice_hi) : context->zero_idx; - - context->create_big_add_gate( - { slice_index, slice_lo_idx, context->zero_idx, slice_hi_idx, -1, 1, 0, (1 << slice_bit_position), 0 }); - - context->create_new_range_constraint(slice_lo_idx, (1ULL << slice_bit_position) - 1); - - if (slice_bit_position != bits_per_limb) { - context->create_new_range_constraint(slice_hi_idx, (1ULL << (bits_per_hi_limb - slice_bit_position)) - 1); - } - + // Example for uint32_t: + // + // |<------ acc[2] ------->||<----------- acc[1] ----------->||<------- acc[0] ------->| + // val: [ 31 30 29 28 27 26 25 24 23 22 21 20 19 18 17 16 15 14 13 12 11 10 9 8 7 6 5 4 3 2 1 0 ] + // ↑ + // [<---- discard ---->][<---------------------- keep ----------------------->] + // + // out: [ 24 23 22 21 20 19 18 17 16 15 14 13 12 11 10 9 8 7 6 5 4 3 2 1 0 ] + // [<- acc[2].lo ->||<----------- acc[1] ----------->||<------- acc[0] ------->| + // + // Suppose the shift is 7, then we must discard the 7 most significant bits of the accumulator, and move + // the remaining bits to the left. The accumulator is split into three parts, so in this case we clearly + // need to split acc[2]. On splitting, we must discard the higher slice of acc[2] and keep the lower slice. + // Thus, the updated uint value will be: + // + // (acc[2].lo << (24 + 7)) + (acc[1] << (12 + 7)) + (acc[0] << 7) + // + // Let us first fetch the accumulator that needs to be sliced. + // We will do so by adjusting the shift from the most-significant bit. + size_t adjusted_shift = width - shift; + const size_t accumulator_index = adjusted_shift / bits_per_limb; + const uint32_t slice_witness_index = accumulators[accumulator_index]; + const field_t acc_to_be_sliced = field_t::from_witness_index(context, slice_witness_index); + + // Now, lets calculate: + // (i) bit position (from lsb) at which we need to slice. + // (ii) number of bits in the slice based on whether it is the highest slice or not. + const bool is_slice_hi = (accumulator_index == num_accumulators() - 1); + const size_t slice_bit_position = adjusted_shift % bits_per_limb; + const uint8_t num_bits_per_limb = is_slice_hi ? bits_in_high_limb : bits_per_limb; + + // We can now slice the accumulator. + field_t slice_lo = acc_to_be_sliced.split_at(slice_bit_position, num_bits_per_limb).first; + + // Now we reconstruct the shifted uint value. std::vector> sublimbs; - sublimbs.emplace_back(field_t::from_witness_index(context, slice_lo_idx) * - field_t(context, 1ULL << ((accumulator_index)*bits_per_limb + shift))); + sublimbs.emplace_back(slice_lo * field_t(context, 1ULL << ((accumulator_index * bits_per_limb) + shift))); field_t coefficient(context, uint64_t(1ULL << shift)); field_t shifter(context, uint64_t(1ULL << bits_per_limb)); for (size_t i = 0; i < accumulator_index; ++i) { - sublimbs.emplace_back(field_t::from_witness_index(context, accumulators[i]) * - field_t(coefficient)); + sublimbs.emplace_back(field_t::from_witness_index(context, accumulators[i]) * coefficient); coefficient *= shifter; } uint32_t result_index = field_t::accumulate(sublimbs).get_witness_index(); uint result(context); result.witness_index = result_index; - result.witness_status = WitnessStatus::WEAK_NORMALIZED; + result.normalize(); return result; } template uint uint::ror(const size_t target_rotation) const { + // Note: width is always a power of two, so we can use bitwise AND. const size_t rotation = target_rotation & (width - 1); const auto rotate = [](const uint256_t input, const uint64_t rot) { @@ -185,64 +193,69 @@ uint uint::ror(const size_t target_rotation) c return uint(context, rotate(additive_constant, rotation)); } - if (witness_status != WitnessStatus::OK) { - normalize(); - } + // Normalize before ror. + normalize(); if (rotation == 0) { return *this; } - const size_t shift = rotation; - uint64_t bits_per_hi_limb; - // last limb will not likely bit `bits_per_limb`. Need to be careful with our range check - if (shift >= ((width / bits_per_limb) * bits_per_limb)) { - bits_per_hi_limb = width % bits_per_limb; - } else { - bits_per_hi_limb = bits_per_limb; - } - const uint64_t slice_bit_position = shift % bits_per_limb; + // Example for uint32_t: + // + // |<------ acc[2] ------>||<----------- acc[1] ----------->||<------- acc[0] ------->| + // val: [ 31 30 29 28 27 26 25 24 23 22 21 20 19 18 17 16 15 14 13 12 11 10 9 8 7 6 5 4 3 2 1 0 ] + // ↑ + // [<--------------- keep --------------->][<-------- right rotate -------->] + // + // out: [ 14 13 12 11 10 9 8 7 6 5 4 3 2 1 0 ] [ 31 30 29 28 27 26 25 24 23 22 21 20 19 18 17 16 15 ] + // [<- acc[1].lo ->||<------- acc[0] ------->| |<------ acc[2] ------>||<----- acc[1].hi ------>] + // + // Suppose the right-rotation is 15 (i.e., rotate = 15), then we must right-rotate the 15 least + // significant bits of the accumulator. The accumulator is split into three parts, so in this case we need to split + // acc[1]. On splitting, we must "rotate" the lower slice of acc[1] and keep the upper slice. Thus, the updated uint + // value will be: + // + // acc[1].hi + (acc[2] << (24 - 15)) + (acc[0] >> (32 - 15)) + (acc[1].lo >> (32 - 15 + 12)) + // + // Let us first fetch the accumlator that needs to be sliced. + size_t shift = rotation; const size_t accumulator_index = shift / bits_per_limb; - const uint32_t slice_index = accumulators[accumulator_index]; - const uint64_t slice_value = uint256_t(context->get_variable(slice_index)).data[0]; + const uint32_t slice_witness_index = accumulators[accumulator_index]; + const field_t acc_to_be_sliced = field_t::from_witness_index(context, slice_witness_index); - const uint64_t slice_lo = slice_value % (1ULL << slice_bit_position); - const uint64_t slice_hi = slice_value >> slice_bit_position; - const uint32_t slice_lo_idx = slice_bit_position ? context->add_variable(slice_lo) : context->zero_idx; - const uint32_t slice_hi_idx = - (slice_bit_position != bits_per_limb) ? context->add_variable(slice_hi) : context->zero_idx; + // Now, lets calculate: + // (i) bit position (from lsb) at which we need to slice. + // (ii) number of bits in the slice based on whether it is the highest slice or not. + const size_t slice_bit_position = shift % bits_per_limb; + const bool is_slice_hi = (accumulator_index == num_accumulators() - 1); + const uint8_t num_bits_per_limb = is_slice_hi ? bits_in_high_limb : bits_per_limb; - context->create_big_add_gate( - { slice_index, slice_lo_idx, context->zero_idx, slice_hi_idx, -1, 1, 0, (1 << slice_bit_position), 0 }); + // Finally, we can slice the accumulator. + auto [slice_lo, slice_hi] = acc_to_be_sliced.split_at(slice_bit_position, num_bits_per_limb); - if (slice_bit_position != 0) { - context->create_new_range_constraint(slice_lo_idx, (1ULL << slice_bit_position) - 1); - } - context->create_new_range_constraint(slice_hi_idx, (1ULL << (bits_per_hi_limb - slice_bit_position)) - 1); + // Now we reconstruct the shifted uint value. std::vector> sublimbs; - sublimbs.emplace_back(field_t::from_witness_index(context, slice_hi_idx)); + sublimbs.emplace_back(slice_hi); const size_t start = accumulator_index + 1; field_t coefficient(context, uint64_t(1ULL << (start * bits_per_limb - shift))); field_t shifter(context, uint64_t(1ULL << bits_per_limb)); for (size_t i = accumulator_index + 1; i < num_accumulators(); ++i) { - sublimbs.emplace_back(field_t::from_witness_index(context, accumulators[i]) * - field_t(coefficient)); + sublimbs.emplace_back(field_t::from_witness_index(context, accumulators[i]) * coefficient); coefficient *= shifter; } coefficient = field_t(context, uint64_t(1ULL << (width - shift))); for (size_t i = 0; i < accumulator_index; ++i) { - sublimbs.emplace_back(field_t::from_witness_index(context, accumulators[i]) * - field_t(coefficient)); + sublimbs.emplace_back(field_t::from_witness_index(context, accumulators[i]) * coefficient); coefficient *= shifter; } - sublimbs.emplace_back(field_t::from_witness_index(context, slice_lo_idx) * field_t(coefficient)); + sublimbs.emplace_back(slice_lo * field_t(coefficient)); uint32_t result_index = field_t::accumulate(sublimbs).get_witness_index(); uint result(context); result.witness_index = result_index; - result.witness_status = WitnessStatus::WEAK_NORMALIZED; + result.normalize(); return result; } @@ -348,8 +361,8 @@ uint uint::logic_operator(const uint& other, c } } + // Since we reconstruct accumulators from the lookup table, we don't need to normalize them here. result.witness_index = lookup[ColumnIdx::C3][0].get_witness_index(); - result.witness_status = WitnessStatus::OK; return result; } diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/uint.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/uint.cpp index 2fbd81f9d756..3cc8762c907d 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/uint.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/uint.cpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: not started, auditors: [], date: YYYY-MM-DD } +// internal: { status: done, auditors: [suyash], date: 2025-07-23 } // external_1: { status: not started, auditors: [], date: YYYY-MM-DD } // external_2: { status: not started, auditors: [], date: YYYY-MM-DD } // ===================== @@ -13,35 +13,33 @@ template std::vector uint::constrain_accumulators(Builder* context, const uint32_t witness_index) const { - const auto res = context->decompose_into_default_range(witness_index, width, bits_per_limb); + std::vector res = context->decompose_into_default_range(witness_index, width, bits_per_limb); return res; } template -uint::uint(const witness_t& witness) - : context(witness.context) - , witness_status(WitnessStatus::OK) +uint::uint(const witness_t& other) + : context(other.context) { - if (witness.witness_index == IS_CONSTANT) { - additive_constant = witness.witness; + if (other.is_constant()) { + additive_constant = other.witness; witness_index = IS_CONSTANT; } else { - accumulators = constrain_accumulators(context, witness.witness_index); - witness_index = witness.witness_index; + accumulators = constrain_accumulators(context, other.witness_index); + witness_index = other.witness_index; } } template -uint::uint(const field_t& value) - : context(value.context) +uint::uint(const field_t& other) + : context(other.context) , additive_constant(0) - , witness_status(WitnessStatus::OK) { - if (value.witness_index == IS_CONSTANT) { - additive_constant = value.additive_constant; + if (other.is_constant()) { + additive_constant = other.additive_constant; witness_index = IS_CONSTANT; } else { - field_t norm = value.normalize(); + field_t norm = other.normalize(); accumulators = constrain_accumulators(context, norm.get_witness_index()); witness_index = norm.get_witness_index(); } @@ -51,7 +49,6 @@ template uint::uint(Builder* builder, const uint256_t& value) : context(builder) , additive_constant(value) - , witness_status(WitnessStatus::OK) , accumulators() , witness_index(IS_CONSTANT) {} @@ -60,7 +57,6 @@ template uint::uint(const uint256_t& value) : context(nullptr) , additive_constant(value) - , witness_status(WitnessStatus::OK) , accumulators() , witness_index(IS_CONSTANT) {} @@ -69,25 +65,32 @@ template uint::uint(const byte_array& other) : context(other.get_context()) , additive_constant(0) - , witness_status(WitnessStatus::WEAK_NORMALIZED) , accumulators() , witness_index(IS_CONSTANT) { - field_t accumulator(context, fr::zero()); + const auto& bytes = other.bytes(); + const size_t num_bytes = bytes.size(); field_t scaling_factor(context, fr::one()); - const auto bytes = other.bytes(); - // TODO JUMP IN STEPS OF TWO - for (size_t i = 0; i < bytes.size(); ++i) { - accumulator = accumulator + scaling_factor * bytes[bytes.size() - 1 - i]; - scaling_factor = scaling_factor * fr(256); + // Collect the bytes in reverse order and scale them appropriately. + std::vector> scaled_bytes; + scaled_bytes.reserve(num_bytes); + for (size_t i = 0; i < num_bytes; ++i) { + scaled_bytes.push_back(bytes[num_bytes - 1 - i] * scaling_factor); + scaling_factor = scaling_factor * fr(256); // Scale by 2^8. } - accumulator = accumulator.normalize(); - if (accumulator.witness_index == IS_CONSTANT) { + field_t accumulator = field_t::accumulate(scaled_bytes); + + // If the accumulator is constant, we set the additive constant. + // Otherwise, we set the witness index. + if (accumulator.is_constant()) { additive_constant = uint256_t(accumulator.additive_constant); } else { witness_index = accumulator.witness_index; } + + // We need to constrain the accumulators, so we normalize here. + normalize(); } template @@ -99,59 +102,66 @@ template uint::uint(Builder* parent_context, const std::vector>& wires) : context(parent_context) , additive_constant(0) - , witness_status(WitnessStatus::WEAK_NORMALIZED) , accumulators() , witness_index(IS_CONSTANT) { - field_t accumulator(context, fr::zero()); field_t scaling_factor(context, fr::one()); - - // TODO JUMP IN STEPS OF TWO - for (size_t i = 0; i < wires.size(); ++i) { - accumulator = accumulator + scaling_factor * field_t(wires[i]); - scaling_factor = scaling_factor + scaling_factor; + const size_t num_wires = wires.size(); + + // Collect the bits and scale them appropriately. + std::vector> scaled_bits; + scaled_bits.reserve(num_wires); + for (size_t i = 0; i < num_wires; ++i) { + scaled_bits.push_back(field_t(wires[i]) * scaling_factor); + scaling_factor = scaling_factor * fr(2); // Scale by 2^1. } - accumulator = accumulator.normalize(); - if (accumulator.witness_index == IS_CONSTANT) { + field_t accumulator = field_t::accumulate(scaled_bits); + + // If the accumulator is constant, we set the additive constant. + // Otherwise, we set the witness index. + if (accumulator.is_constant()) { additive_constant = uint256_t(accumulator.additive_constant); } else { witness_index = accumulator.witness_index; } + + // We need to constrain the accumulators, so we normalize here. + normalize(); } template uint::uint(const uint& other) : context(other.context) , additive_constant(other.additive_constant) - , witness_status(other.witness_status) , accumulators(other.accumulators) , witness_index(other.witness_index) {} template -uint::uint(uint&& other) +uint::uint(uint&& other) noexcept : context(other.context) , additive_constant(other.additive_constant) - , witness_status(other.witness_status) , accumulators(other.accumulators) , witness_index(other.witness_index) {} template uint& uint::operator=(const uint& other) { + if (this == &other) { + return *this; + } context = other.context; additive_constant = other.additive_constant; - witness_status = other.witness_status; accumulators = other.accumulators; witness_index = other.witness_index; return *this; } -template uint& uint::operator=(uint&& other) +template +uint& uint::operator=(uint&& other) noexcept { context = other.context; additive_constant = other.additive_constant; - witness_status = other.witness_status; accumulators = other.accumulators; witness_index = other.witness_index; return *this; @@ -177,10 +187,8 @@ template uint uint uint256_t uint::ge if (!context || is_constant()) { return additive_constant; } - return (uint256_t(context->get_variable(witness_index))) & MASK; -} - -template uint256_t uint::get_unbounded_value() const -{ - if (!context || is_constant()) { - return additive_constant; - } - return (uint256_t(context->get_variable(witness_index))); -} -template bool_t uint::at(const size_t bit_index) const -{ - if (is_constant()) { - return bool_t(context, get_value().get_bit(bit_index)); - } - if (witness_status != WitnessStatus::OK) { - normalize(); - } + const uint256_t witness_value = context->get_variable(witness_index); + ASSERT(witness_value.get_msb() < width, "uint::get_value(): witness value exceeds type width"); - const uint64_t slice_bit_position = bit_index % bits_per_limb; - - const uint32_t slice_index = accumulators[bit_index / bits_per_limb]; - const uint64_t slice_value = uint256_t(context->get_variable(slice_index)).data[0]; - - const uint64_t slice_lo = slice_value % (1ULL << slice_bit_position); - const uint64_t bit_value = (slice_value >> slice_bit_position) & 1ULL; - const uint64_t slice_hi = slice_value >> (slice_bit_position + 1); - - const uint32_t slice_lo_idx = slice_bit_position ? context->add_variable(slice_lo) : context->zero_idx; - const uint32_t bit_idx = context->add_variable(bit_value); - const uint32_t slice_hi_idx = - (slice_bit_position + 1 != bits_per_limb) ? context->add_variable(slice_hi) : context->zero_idx; - - context->create_big_add_gate({ slice_index, - slice_lo_idx, - bit_idx, - slice_hi_idx, - -1, - 1, - (1 << slice_bit_position), - (1 << (slice_bit_position + 1)), - 0 }); - - if (slice_bit_position != 0) { - context->create_new_range_constraint(slice_lo_idx, (1ULL << slice_bit_position) - 1); - } - if (slice_bit_position + 1 != bits_per_limb) { - context->create_new_range_constraint(slice_hi_idx, (1ULL << (bits_per_limb - (slice_bit_position + 1))) - 1); - } - bool_t result = witness_t(context, bit_value); - return result; + return witness_value & MASK; } template class uint; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/uint.fuzzer.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/uint.fuzzer.hpp index fd3e3fbee49e..4d23091f2cee 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/uint.fuzzer.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/uint.fuzzer.hpp @@ -69,7 +69,6 @@ template class UintFuzzBase { AND, OR, XOR, - GET_BIT, SHL, SHR, ROL, @@ -140,7 +139,6 @@ template class UintFuzzBase { out = static_cast(rng.next() & 0xff); return { .id = instruction_opcode, .arguments.threeArgs = { .in1 = in1, .in2 = in2, .out = out } }; break; - case OPCODE::GET_BIT: case OPCODE::SHL: case OPCODE::SHR: case OPCODE::ROL: @@ -204,7 +202,6 @@ template class UintFuzzBase { PUT_RANDOM_BYTE_IF_LUCKY(instruction.arguments.threeArgs.in2) PUT_RANDOM_BYTE_IF_LUCKY(instruction.arguments.threeArgs.out) break; - case OPCODE::GET_BIT: case OPCODE::SHL: case OPCODE::SHR: case OPCODE::ROL: @@ -241,7 +238,6 @@ template class UintFuzzBase { static constexpr size_t AND = 3; static constexpr size_t OR = 3; static constexpr size_t XOR = 3; - static constexpr size_t GET_BIT = 10; static constexpr size_t SHL = 10; static constexpr size_t SHR = 10; static constexpr size_t ROL = 10; @@ -276,9 +272,8 @@ template class UintFuzzBase { return { .id = static_cast(opcode), .arguments.threeArgs = { .in1 = *Data, .in2 = *(Data + 1), .out = *(Data + 2) } }; } - if constexpr (opcode == Instruction::OPCODE::GET_BIT || opcode == Instruction::OPCODE::SHL || - opcode == Instruction::OPCODE::SHR || opcode == Instruction::OPCODE::ROL || - opcode == Instruction::OPCODE::ROR) { + if constexpr (opcode == Instruction::OPCODE::SHL || opcode == Instruction::OPCODE::SHR || + opcode == Instruction::OPCODE::ROL || opcode == Instruction::OPCODE::ROR) { return Instruction{ .id = static_cast(opcode), .arguments.bitArgs = { .in = *Data, .out = *(Data + 1), .bit = *(Data + 2) } }; } @@ -320,8 +315,7 @@ template class UintFuzzBase { *(Data + 2) = instruction.arguments.threeArgs.in2; *(Data + 3) = instruction.arguments.threeArgs.out; } - if constexpr (instruction_opcode == Instruction::OPCODE::GET_BIT || - instruction_opcode == Instruction::OPCODE::SHL || + if constexpr (instruction_opcode == Instruction::OPCODE::SHL || instruction_opcode == Instruction::OPCODE::SHR || instruction_opcode == Instruction::OPCODE::ROL || instruction_opcode == Instruction::OPCODE::ROR) { @@ -365,48 +359,6 @@ template class UintFuzzBase { return static_cast(v >> bits); } } - template static T get_bit(const T v, const size_t bit) - { - if (bit >= sizeof(T) * 8) { - return 0; - } else { - return (v & (uint64_t(1) << bit)) ? 1 : 0; - } - } - /* wrapper for uint::at which ensures the context of - * the return value has been set - */ - template static bool_t at(const T& v, const size_t bit_index) - { - const auto ret = v.at(bit_index); - - if (ret.get_context() != v.get_context()) { - std::cerr << "Context of return bool_t not set" << std::endl; - abort(); - } - - return ret; - } - template static T get_bit(Builder* builder, const T& v, const size_t bit) - { - return T(builder, std::vector{ at<>(v, bit) }); - } - template static std::vector to_bit_vector(const T& v) - { - std::vector bits; - for (size_t i = 0; i < v.get_width(); i++) { - bits.push_back(at<>(v, i)); - } - return bits; - } - template static std::array to_bit_array(const T& v) - { - std::array bits; - for (size_t i = 0; i < T::width; i++) { - bits[i] = at<>(v, i); - } - return bits; - } template static uint256_t get_value(const T& v) { const auto ret = v.get_value(); @@ -761,17 +713,6 @@ template class UintFuzzBase { abort(); } } - ExecutionHandler get_bit(Builder* builder, const size_t bit) const - { - return ExecutionHandler(Reference(this->get_bit(this->ref.v8, bit), - this->get_bit(this->ref.v16, bit), - this->get_bit(this->ref.v32, bit), - this->get_bit(this->ref.v64, bit)), - Uint(this->get_bit(builder, this->uint.v8, bit), - this->get_bit(builder, this->uint.v16, bit), - this->get_bit(builder, this->uint.v32, bit), - this->get_bit(builder, this->uint.v64, bit))); - } ExecutionHandler shl(const size_t bits) const { const Reference ref_result(shl(this->ref.v8, bits), @@ -862,7 +803,7 @@ template class UintFuzzBase { /* Explicit re-instantiation using the various constructors */ ExecutionHandler set(Builder* builder) const { - switch (VarianceRNG.next() % 7) { + switch (VarianceRNG.next() % 5) { case 0: return ExecutionHandler(this->ref, Uint(uint_8_t(this->uint.v8), @@ -888,18 +829,6 @@ template class UintFuzzBase { uint_32_t(this->to_byte_array(this->uint.v32)), uint_64_t(this->to_byte_array(this->uint.v64)))); case 4: - return ExecutionHandler(this->ref, - Uint(uint_8_t(builder, this->to_bit_vector(this->uint.v8)), - uint_16_t(builder, this->to_bit_vector(this->uint.v16)), - uint_32_t(builder, this->to_bit_vector(this->uint.v32)), - uint_64_t(builder, this->to_bit_vector(this->uint.v64)))); - case 5: - return ExecutionHandler(this->ref, - Uint(uint_8_t(builder, this->to_bit_array(this->uint.v8)), - uint_16_t(builder, this->to_bit_array(this->uint.v16)), - uint_32_t(builder, this->to_bit_array(this->uint.v32)), - uint_64_t(builder, this->to_bit_array(this->uint.v64)))); - case 6: return ExecutionHandler(this->ref, Uint(uint_8_t(builder, this->ref.v8), uint_16_t(builder, this->ref.v16), @@ -1164,34 +1093,6 @@ template class UintFuzzBase { } return 0; }; - /** - * @brief Execute the GET_BIT instruction - * - * @param builder - * @param stack - * @param instruction - * @return if everything is ok, 1 if we should stop execution, since an expected error was encountered - */ - static inline size_t execute_GET_BIT(Builder* builder, - std::vector& stack, - Instruction& instruction) - { - if (stack.size() == 0) { - return 1; - } - size_t first_index = instruction.arguments.bitArgs.in % stack.size(); - size_t output_index = instruction.arguments.bitArgs.out; - const uint8_t bit = instruction.arguments.bitArgs.bit; - ExecutionHandler result; - result = stack[first_index].get_bit(builder, bit); - // If the output index is larger than the number of elements in stack, append - if (output_index >= stack.size()) { - stack.push_back(result); - } else { - stack[output_index] = result; - } - return 0; - }; /** * @brief Execute the left-shift operator instruction * diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/uint.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/uint.hpp index 7c68f3de436a..2dcb2957737b 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/uint.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/uint.hpp @@ -1,5 +1,5 @@ // === AUDIT STATUS === -// internal: { status: not started, auditors: [], date: YYYY-MM-DD } +// internal: { status: done, auditors: [suyash], date: 2025-07-23 } // external_1: { status: not started, auditors: [], date: YYYY-MM-DD } // external_2: { status: not started, auditors: [], date: YYYY-MM-DD } // ===================== @@ -18,6 +18,9 @@ template class uint { using FF = typename Builder::FF; static constexpr size_t width = sizeof(Native) * 8; + static_assert(width == 8 || width == 16 || width == 32 || width == 64, + "unsupported uint width, supported uint widths are: 8, 16, 32, and 64 bits."); + uint(const witness_t& other); uint(const field_t& other); uint(const uint256_t& value = 0); @@ -30,26 +33,31 @@ template class uint { : uint(static_cast(v)) {} - std::vector constrain_accumulators(Builder* ctx, const uint32_t witness_index) const; + std::vector constrain_accumulators(Builder* context, const uint32_t witness_index) const; static constexpr size_t bits_per_limb = 12; + static constexpr size_t bits_in_high_limb = width % bits_per_limb == 0 ? bits_per_limb : width % bits_per_limb; static constexpr size_t num_accumulators() { return (width + bits_per_limb - 1) / bits_per_limb; } uint(const uint& other); - uint(uint&& other); + uint(uint&& other) noexcept; + + ~uint() = default; uint& operator=(const uint& other); - uint& operator=(uint&& other); + uint& operator=(uint&& other) noexcept; explicit operator byte_array() const; explicit operator field_t() const; + // Arithmetic operations uint operator+(const uint& other) const; uint operator-(const uint& other) const; uint operator*(const uint& other) const; uint operator/(const uint& other) const; uint operator%(const uint& other) const; + // Bitwise operations uint operator&(const uint& other) const; uint operator^(const uint& other) const; uint operator|(const uint& other) const; @@ -63,6 +71,7 @@ template class uint { uint ror(const uint256_t target_rotation) const { return ror(static_cast(target_rotation.data[0])); } uint rol(const uint256_t target_rotation) const { return rol(static_cast(target_rotation.data[0])); } + // Comparison operations bool_t operator>(const uint& other) const; bool_t operator<(const uint& other) const; bool_t operator>=(const uint& other) const; @@ -71,6 +80,7 @@ template class uint { bool_t operator!=(const uint& other) const; bool_t operator!() const; + // Assignment operators uint operator+=(const uint& other) { *this = operator+(other); @@ -124,6 +134,7 @@ template class uint { return *this; } + // Helper functions uint normalize() const; uint256_t get_value() const; @@ -131,8 +142,6 @@ template class uint { bool is_constant() const { return witness_index == IS_CONSTANT; } Builder* get_context() const { return context; } - bool_t at(const size_t bit_index) const; - size_t get_width() const { return width; } uint32_t get_witness_index() const { return witness_index; } @@ -140,17 +149,15 @@ template class uint { uint256_t get_additive_constant() const { return additive_constant; } std::vector get_accumulators() const { return accumulators; } - uint256_t get_unbounded_value() const; protected: Builder* context; - enum WitnessStatus { OK, NOT_NORMALIZED, WEAK_NORMALIZED }; - mutable uint256_t additive_constant; - mutable WitnessStatus witness_status; // N.B. Not an accumulator! Contains 6-bit slices of input + // `accumulators` are used to store 6-bit slices of the value of the uint. This is done to + // range-constrain the uint by constraining individual slices. mutable std::vector accumulators; mutable uint32_t witness_index; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/uint.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/uint.test.cpp index 6b4d591035e8..903b16e4b311 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/uint.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/uint/uint.test.cpp @@ -189,8 +189,6 @@ template class stdlib_uint : public testing::Test { witness_ct(&builder, true), }); - EXPECT_EQ(a.at(0).get_value(), false); - EXPECT_EQ(a.at(7).get_value(), true); EXPECT_EQ(static_cast(a.get_value()), 128U); } @@ -1035,6 +1033,13 @@ template class stdlib_uint : public testing::Test { uint_ct b_shift = uint_ct(&builder, const_b); uint_ct c = a + a_shift; uint_ct d = b + b_shift; + + // If both dividend and divisor are constants and divisor is zero, we expect an exception to be thrown. + if (divisor_zero && (lhs_constant && rhs_constant)) { + EXPECT_THROW_OR_ABORT(c / d, "divide by zero with constant dividend and divisor"); + return; + } + uint_ct e = c / d; e = e.normalize(); @@ -1730,39 +1735,6 @@ template class stdlib_uint : public testing::Test { bool proof_result = CircuitChecker::check(builder); EXPECT_EQ(proof_result, true); } - - /** - * @brief Test the function uint_ct::at used to extract bits. - */ - static void test_at() - { - Builder builder = Builder(); - - const auto bit_test = [&builder](const bool is_constant) { - // construct a sum of uint_ct's, where at least one is a constant, - // and validate its correctness bitwise - uint_native const_a = get_random(); - uint_native a_val = get_random(); - uint_native c_val = const_a + a_val; - uint_ct a = is_constant ? uint_ct(&builder, a_val) : witness_ct(&builder, a_val); - uint_ct a_shift = uint_ct(&builder, const_a); - uint_ct c = a + a_shift; - for (size_t i = 0; i < uint_native_width; ++i) { - bool_ct result = c.at(i); - bool expected = (((c_val >> i) & 1UL) == 1UL) ? true : false; - EXPECT_EQ(result.get_value(), expected); - EXPECT_EQ(result.get_context(), c.get_context()); - } - }; - - bit_test(false); - bit_test(true); - - printf("builder gates = %zu\n", builder.get_estimated_num_finalized_gates()); - - bool proof_result = CircuitChecker::check(builder); - EXPECT_EQ(proof_result, true); - } }; // Define the test types for all combinations @@ -1938,7 +1910,3 @@ TYPED_TEST(stdlib_uint, test_rol) { TestFixture::test_rol(); } -TYPED_TEST(stdlib_uint, test_at) -{ - TestFixture::test_at(); -} diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/witness/witness.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/witness/witness.hpp index 3c42571feecd..902023130b93 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/witness/witness.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/witness/witness.hpp @@ -49,6 +49,8 @@ template class witness_t { return out; } + bool is_constant() const { return witness_index == IS_CONSTANT; } + bb::fr witness; uint32_t witness_index = IS_CONSTANT; Builder* context = nullptr; diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp index 6db99da01004..09d0e2057124 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.cpp @@ -510,18 +510,11 @@ void UltraCircuitBuilder_::create_balanced_add_gate(const add_qu } check_selector_length_consistency(); ++this->num_gates; - // Why 3? TODO: return to this - // The purpose of this gate is to do enable lazy 32-bit addition. - // Consider a + b = c mod 2^32 - // We want the 4th wire to represent the quotient: - // w1 + w2 = w4 * 2^32 + w3 - // If we allow this overflow 'flag' to range from 0 to 3, instead of 0 to 1, - // we can get away with chaining a few addition operations together with basic add gates, - // before having to use this gate. - // (N.B. a larger value would be better, the value '3' is for Turbo backwards compatibility. - // In Turbo this method uses a custom gate, - // where we were limited to a 2-bit range check by the degree of the custom gate identity. - create_new_range_constraint(in.d, 3); + + // Range constrain the 4-th wire to {0, 1}. Since the inputs being added never exceed (2^x - 1) + // during uintx arithmetic, we can safely use a 1-bit range check here. In other words, we do not + // allow lazy uintx addition. + create_new_range_constraint(in.d, 1); } /** * @brief Create a multiplication gate with q_m * a * b + q_3 * c + q_const = 0