From 7d1176a84d7a44df6fee91df3007e53e88eb8776 Mon Sep 17 00:00:00 2001 From: suyash67 Date: Sun, 8 Jun 2025 08:15:23 +0200 Subject: [PATCH 01/34] remove (unused) modulus from non_native_field_witnesses. --- .../circuit_checker/ultra_circuit_builder.test.cpp | 4 ++-- .../barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp | 3 --- .../stdlib_circuit_builders/ultra_circuit_builder.hpp | 1 - .../cpp/src/barretenberg/ultra_honk/ultra_honk.test.cpp | 2 +- 4 files changed, 3 insertions(+), 7 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_builder.test.cpp b/barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_builder.test.cpp index 00606c511b52..a5b23a91e408 100644 --- a/barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_builder.test.cpp +++ b/barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_builder.test.cpp @@ -613,7 +613,7 @@ TEST(UltraCircuitBuilder, NonNativeFieldMultiplication) const auto r_indices = get_limb_witness_indices(split_into_limbs(uint256_t(r))); non_native_field_witnesses inputs{ - a_indices, b_indices, q_indices, r_indices, modulus_limbs, fr(uint256_t(modulus)), + a_indices, b_indices, q_indices, r_indices, modulus_limbs, }; const auto [lo_1_idx, hi_1_idx] = builder.evaluate_non_native_field_multiplication(inputs); builder.range_constrain_two_limbs(lo_1_idx, hi_1_idx, 70, 70); @@ -671,7 +671,7 @@ TEST(UltraCircuitBuilder, NonNativeFieldMultiplicationSortCheck) const auto r_indices = get_limb_witness_indices(split_into_limbs(uint256_t(r))); non_native_field_witnesses inputs{ - a_indices, b_indices, q_indices, r_indices, modulus_limbs, fr(uint256_t(modulus)), + a_indices, b_indices, q_indices, r_indices, modulus_limbs, }; const auto [lo_1_idx, hi_1_idx] = builder.evaluate_non_native_field_multiplication(inputs); builder.range_constrain_two_limbs(lo_1_idx, hi_1_idx, 70, 70); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp index e28a11b3b4d3..e4b712efec12 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp @@ -2270,7 +2270,6 @@ void bigfield::unsafe_evaluate_multiply_add(const bigfield& input_le remainder_limbs[3].get_normalized_witness_index(), }, { neg_modulus_limbs[0], neg_modulus_limbs[1], neg_modulus_limbs[2], neg_modulus_limbs[3] }, - modulus, }; // N.B. this method DOES NOT evaluate the prime field component of the non-native field mul @@ -2531,7 +2530,6 @@ void bigfield::unsafe_evaluate_multiple_multiply_add(const std::vect ctx->zero_idx, }, { 0, 0, 0, 0 }, - modulus, }; const auto [lo_2_idx, hi_2_idx] = ctx->queue_partial_non_native_field_multiplication(mul_witnesses); @@ -2623,7 +2621,6 @@ void bigfield::unsafe_evaluate_multiple_multiply_add(const std::vect remainder_limbs[3].get_normalized_witness_index(), }, { neg_modulus_limbs[0], neg_modulus_limbs[1], neg_modulus_limbs[2], neg_modulus_limbs[3] }, - modulus, }; const auto [lo_1_idx, hi_1_idx] = ctx->evaluate_non_native_field_multiplication(witnesses); diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp index ee35f9c784a2..d26274d5756c 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp @@ -28,7 +28,6 @@ template struct non_native_field_witnesses { std::array q; std::array r; std::array neg_modulus; - FF modulus; }; template diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/ultra_honk.test.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/ultra_honk.test.cpp index 3c3b06da401d..c5d3461d3169 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/ultra_honk.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/ultra_honk.test.cpp @@ -882,7 +882,7 @@ TYPED_TEST(UltraHonkTests, NonNativeFieldMultiplication) const auto r_indices = get_limb_witness_indices(split_into_limbs(uint256_t(r))); non_native_field_witnesses inputs{ - a_indices, b_indices, q_indices, r_indices, modulus_limbs, fr(uint256_t(modulus)), + a_indices, b_indices, q_indices, r_indices, modulus_limbs, }; const auto [lo_1_idx, hi_1_idx] = circuit_builder.evaluate_non_native_field_multiplication(inputs); circuit_builder.range_constrain_two_limbs(lo_1_idx, hi_1_idx, 70, 70); From a3c100187ac222156bd4bcde23f8b2de4c826f05 Mon Sep 17 00:00:00 2001 From: suyash67 Date: Sun, 8 Jun 2025 09:02:45 +0200 Subject: [PATCH 02/34] simplify condition for l and r constant_to_fixed_wit. --- .../stdlib/primitives/bigfield/bigfield_impl.hpp | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp index e4b712efec12..0632e66a7a79 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp @@ -2490,16 +2490,10 @@ void bigfield::unsafe_evaluate_multiple_multiply_add(const std::vect std::vector> prime_limb_accumulator; for (size_t i = 0; i < num_multiplications; ++i) { - if (i == 0 && left[0].is_constant()) { - left[0] = convert_constant_to_fixed_witness(left[0]); - } - if (i == 0 && right[0].is_constant()) { - right[0] = convert_constant_to_fixed_witness(right[0]); - } - if (i > 0 && left[i].is_constant()) { + if (left[i].is_constant()) { left[i] = convert_constant_to_fixed_witness(left[i]); } - if (i > 0 && right[i].is_constant()) { + if (right[i].is_constant()) { right[i] = convert_constant_to_fixed_witness(right[i]); } From 89b88113a432ff4a480c8286f51ab41faa7d8d4c Mon Sep 17 00:00:00 2001 From: suyash67 Date: Sun, 8 Jun 2025 09:47:11 +0200 Subject: [PATCH 03/34] remove redundant static casts (prob were there because we didn't have those witness indices as uint32_t in the past). --- .../stdlib_circuit_builders/ultra_circuit_builder.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 98f31ea00df2..a10fb6518d69 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 @@ -1893,7 +1893,7 @@ template void UltraCircuitBuilder_::pr // iterate over the cached items and create constraints for (const auto& input : cached_partial_non_native_field_multiplications) { - blocks.aux.populate_wires(input.a[1], input.b[1], this->zero_idx, static_cast(input.lo_0)); + blocks.aux.populate_wires(input.a[1], input.b[1], this->zero_idx, input.lo_0); apply_aux_selectors(AUX_SELECTORS::NON_NATIVE_FIELD_1); ++this->num_gates; @@ -1901,11 +1901,11 @@ template void UltraCircuitBuilder_::pr apply_aux_selectors(AUX_SELECTORS::NON_NATIVE_FIELD_2); ++this->num_gates; - blocks.aux.populate_wires(input.a[2], input.b[2], this->zero_idx, static_cast(input.hi_0)); + blocks.aux.populate_wires(input.a[2], input.b[2], this->zero_idx, input.hi_0); apply_aux_selectors(AUX_SELECTORS::NON_NATIVE_FIELD_3); ++this->num_gates; - blocks.aux.populate_wires(input.a[1], input.b[1], this->zero_idx, static_cast(input.hi_1)); + blocks.aux.populate_wires(input.a[1], input.b[1], this->zero_idx, input.hi_1); apply_aux_selectors(AUX_SELECTORS::NONE); ++this->num_gates; } From 6fdf16affd94cc465fc0a9bbccd53ed9c9b1be66 Mon Sep 17 00:00:00 2001 From: suyash67 Date: Mon, 9 Jun 2025 10:05:19 +0200 Subject: [PATCH 04/34] modify the input to queue_partial_non_native_field_multiplication as non_native_field_witnesses has members that are unused. --- .../stdlib/primitives/bigfield/bigfield_impl.hpp | 13 ------------- .../ultra_circuit_builder.cpp | 2 +- .../ultra_circuit_builder.hpp | 9 ++++++++- 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp index 0632e66a7a79..385cb99c56d3 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp @@ -2511,19 +2511,6 @@ void bigfield::unsafe_evaluate_multiple_multiply_add(const std::vect right[i].binary_basis_limbs[2].element.get_normalized_witness_index(), right[i].binary_basis_limbs[3].element.get_normalized_witness_index(), }, - { - ctx->zero_idx, - ctx->zero_idx, - ctx->zero_idx, - ctx->zero_idx, - }, - { - ctx->zero_idx, - ctx->zero_idx, - ctx->zero_idx, - ctx->zero_idx, - }, - { 0, 0, 0, 0 }, }; const auto [lo_2_idx, hi_2_idx] = ctx->queue_partial_non_native_field_multiplication(mul_witnesses); 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 a10fb6518d69..d5df7533aa5d 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 @@ -1921,7 +1921,7 @@ template void UltraCircuitBuilder_::pr template std::array UltraCircuitBuilder_::queue_partial_non_native_field_multiplication( - const non_native_field_witnesses& input) + const non_native_partial_multiplication_witnesses& input) { std::array a{ diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp index d26274d5756c..1ffdbc607dc6 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp @@ -30,6 +30,12 @@ template struct non_native_field_witnesses { std::array neg_modulus; }; +template struct non_native_partial_multiplication_witnesses { + // first 4 array elements = limbs + std::array a; + std::array b; +}; + template class UltraCircuitBuilder_ : public CircuitBuilderBase { public: @@ -818,7 +824,8 @@ class UltraCircuitBuilder_ : public CircuitBuilderBase decompose_non_native_field_double_width_limb( const uint32_t limb_idx, const size_t num_limb_bits = (2 * DEFAULT_NON_NATIVE_FIELD_LIMB_BITS)); std::array evaluate_non_native_field_multiplication(const non_native_field_witnesses& input); - std::array queue_partial_non_native_field_multiplication(const non_native_field_witnesses& input); + std::array queue_partial_non_native_field_multiplication( + const non_native_partial_multiplication_witnesses& input); using scaled_witness = std::pair; using add_simple = std::tuple; std::array evaluate_non_native_field_subtraction(add_simple limb0, From f8415b7e976364650359d016097f219e89c4d0a6 Mon Sep 17 00:00:00 2001 From: suyash67 Date: Mon, 9 Jun 2025 10:08:39 +0200 Subject: [PATCH 05/34] fix bigfield use of queue_partial_non_native_field_multiplication --- .../barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp index 385cb99c56d3..5ca424de7f35 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp @@ -2498,7 +2498,7 @@ void bigfield::unsafe_evaluate_multiple_multiply_add(const std::vect } if (i > 0) { - bb::non_native_field_witnesses mul_witnesses = { + bb::non_native_partial_multiplication_witnesses mul_witnesses = { { left[i].binary_basis_limbs[0].element.get_normalized_witness_index(), left[i].binary_basis_limbs[1].element.get_normalized_witness_index(), From d5b089d673d27771dc522ee99a7aa3989556577a Mon Sep 17 00:00:00 2001 From: suyash67 Date: Mon, 9 Jun 2025 10:19:19 +0200 Subject: [PATCH 06/34] rename non_native_field_witnesses to non_native_multiplication_witnesses. --- .../circuit_checker/ultra_circuit_builder.test.cpp | 4 ++-- .../stdlib/primitives/bigfield/bigfield_impl.hpp | 4 ++-- .../stdlib_circuit_builders/ultra_circuit_builder.cpp | 2 +- .../stdlib_circuit_builders/ultra_circuit_builder.hpp | 5 +++-- .../cpp/src/barretenberg/ultra_honk/ultra_honk.test.cpp | 2 +- 5 files changed, 9 insertions(+), 8 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_builder.test.cpp b/barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_builder.test.cpp index a5b23a91e408..3019532f3b81 100644 --- a/barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_builder.test.cpp +++ b/barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_builder.test.cpp @@ -612,7 +612,7 @@ TEST(UltraCircuitBuilder, NonNativeFieldMultiplication) const auto q_indices = get_limb_witness_indices(split_into_limbs(uint256_t(q))); const auto r_indices = get_limb_witness_indices(split_into_limbs(uint256_t(r))); - non_native_field_witnesses inputs{ + non_native_multiplication_witnesses inputs{ a_indices, b_indices, q_indices, r_indices, modulus_limbs, }; const auto [lo_1_idx, hi_1_idx] = builder.evaluate_non_native_field_multiplication(inputs); @@ -670,7 +670,7 @@ TEST(UltraCircuitBuilder, NonNativeFieldMultiplicationSortCheck) const auto q_indices = get_limb_witness_indices(split_into_limbs(uint256_t(q))); const auto r_indices = get_limb_witness_indices(split_into_limbs(uint256_t(r))); - non_native_field_witnesses inputs{ + non_native_multiplication_witnesses inputs{ a_indices, b_indices, q_indices, r_indices, modulus_limbs, }; const auto [lo_1_idx, hi_1_idx] = builder.evaluate_non_native_field_multiplication(inputs); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp index 5ca424de7f35..1dc5c5bb3bbd 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp @@ -2244,7 +2244,7 @@ void bigfield::unsafe_evaluate_multiply_add(const bigfield& input_le }; field_t remainder_prime_limb = field_t::accumulate(prime_limb_accumulator); - bb::non_native_field_witnesses witnesses{ + bb::non_native_multiplication_witnesses witnesses{ { left.binary_basis_limbs[0].element.get_normalized_witness_index(), left.binary_basis_limbs[1].element.get_normalized_witness_index(), @@ -2576,7 +2576,7 @@ void bigfield::unsafe_evaluate_multiple_multiply_add(const std::vect }; field_t remainder_prime_limb = field_t::accumulate(prime_limb_accumulator); - bb::non_native_field_witnesses witnesses{ + bb::non_native_multiplication_witnesses witnesses{ { left[0].binary_basis_limbs[0].element.get_normalized_witness_index(), left[0].binary_basis_limbs[1].element.get_normalized_witness_index(), 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 d5df7533aa5d..34acdfdc034b 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 @@ -1692,7 +1692,7 @@ std::array UltraCircuitBuilder_::decompose_non_nati **/ template std::array UltraCircuitBuilder_::evaluate_non_native_field_multiplication( - const non_native_field_witnesses& input) + const non_native_multiplication_witnesses& input) { std::array a{ diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp index 1ffdbc607dc6..87174478c630 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp @@ -21,7 +21,7 @@ namespace bb { -template struct non_native_field_witnesses { +template struct non_native_multiplication_witnesses { // first 4 array elements = limbs std::array a; std::array b; @@ -823,7 +823,8 @@ class UltraCircuitBuilder_ : public CircuitBuilderBase decompose_non_native_field_double_width_limb( const uint32_t limb_idx, const size_t num_limb_bits = (2 * DEFAULT_NON_NATIVE_FIELD_LIMB_BITS)); - std::array evaluate_non_native_field_multiplication(const non_native_field_witnesses& input); + std::array evaluate_non_native_field_multiplication( + const non_native_multiplication_witnesses& input); std::array queue_partial_non_native_field_multiplication( const non_native_partial_multiplication_witnesses& input); using scaled_witness = std::pair; diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/ultra_honk.test.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/ultra_honk.test.cpp index c5d3461d3169..48d70032079c 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/ultra_honk.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/ultra_honk.test.cpp @@ -881,7 +881,7 @@ TYPED_TEST(UltraHonkTests, NonNativeFieldMultiplication) const auto q_indices = get_limb_witness_indices(split_into_limbs(uint256_t(q))); const auto r_indices = get_limb_witness_indices(split_into_limbs(uint256_t(r))); - non_native_field_witnesses inputs{ + non_native_multiplication_witnesses inputs{ a_indices, b_indices, q_indices, r_indices, modulus_limbs, }; const auto [lo_1_idx, hi_1_idx] = circuit_builder.evaluate_non_native_field_multiplication(inputs); From d1909f93a57b837ee267094be36a5210bf8dda4d Mon Sep 17 00:00:00 2001 From: suyash67 Date: Mon, 9 Jun 2025 11:32:40 +0200 Subject: [PATCH 07/34] define a fn to get witness indices of limbs, and use it in evaluate_non_native_* calls. --- .../stdlib/primitives/bigfield/bigfield.hpp | 14 +++++ .../primitives/bigfield/bigfield_impl.hpp | 56 +++---------------- 2 files changed, 22 insertions(+), 48 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp index a7c4c2d9e761..f46eaddd7433 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp @@ -890,6 +890,20 @@ template class bigfield { static_assert(PROHIBITED_LIMB_BITS < MAXIMUM_LIMB_SIZE_THAT_WOULDNT_OVERFLOW); private: + /** + * @brief Get the witness indices of the (normalized) binary basis limbs + * + * @return Witness indices of the binary basis limbs + */ + std::array get_binary_basis_limb_witness_indices() const + { + std::array limb_witness_indices; + for (size_t i = 0; i < NUM_LIMBS; i++) { + limb_witness_indices[i] = binary_basis_limbs[i].element.get_normalized_witness_index(); + } + return limb_witness_indices; + } + /** * @brief Compute the quotient and remainder values for dividing (a * b + (to_add[0] + ... + to_add[-1])) with p * diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp index 1dc5c5bb3bbd..5eb34e7ed4a0 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp @@ -2245,24 +2245,9 @@ void bigfield::unsafe_evaluate_multiply_add(const bigfield& input_le field_t remainder_prime_limb = field_t::accumulate(prime_limb_accumulator); bb::non_native_multiplication_witnesses witnesses{ - { - left.binary_basis_limbs[0].element.get_normalized_witness_index(), - left.binary_basis_limbs[1].element.get_normalized_witness_index(), - left.binary_basis_limbs[2].element.get_normalized_witness_index(), - left.binary_basis_limbs[3].element.get_normalized_witness_index(), - }, - { - to_mul.binary_basis_limbs[0].element.get_normalized_witness_index(), - to_mul.binary_basis_limbs[1].element.get_normalized_witness_index(), - to_mul.binary_basis_limbs[2].element.get_normalized_witness_index(), - to_mul.binary_basis_limbs[3].element.get_normalized_witness_index(), - }, - { - quotient.binary_basis_limbs[0].element.get_normalized_witness_index(), - quotient.binary_basis_limbs[1].element.get_normalized_witness_index(), - quotient.binary_basis_limbs[2].element.get_normalized_witness_index(), - quotient.binary_basis_limbs[3].element.get_normalized_witness_index(), - }, + left.get_binary_basis_limb_witness_indices(), + to_mul.get_binary_basis_limb_witness_indices(), + quotient.get_binary_basis_limb_witness_indices(), { remainder_limbs[0].get_normalized_witness_index(), remainder_limbs[1].get_normalized_witness_index(), @@ -2499,18 +2484,8 @@ void bigfield::unsafe_evaluate_multiple_multiply_add(const std::vect if (i > 0) { bb::non_native_partial_multiplication_witnesses mul_witnesses = { - { - left[i].binary_basis_limbs[0].element.get_normalized_witness_index(), - left[i].binary_basis_limbs[1].element.get_normalized_witness_index(), - left[i].binary_basis_limbs[2].element.get_normalized_witness_index(), - left[i].binary_basis_limbs[3].element.get_normalized_witness_index(), - }, - { - right[i].binary_basis_limbs[0].element.get_normalized_witness_index(), - right[i].binary_basis_limbs[1].element.get_normalized_witness_index(), - right[i].binary_basis_limbs[2].element.get_normalized_witness_index(), - right[i].binary_basis_limbs[3].element.get_normalized_witness_index(), - }, + left[i].get_binary_basis_limb_witness_indices(), + right[i].get_binary_basis_limb_witness_indices(), }; const auto [lo_2_idx, hi_2_idx] = ctx->queue_partial_non_native_field_multiplication(mul_witnesses); @@ -2577,24 +2552,9 @@ void bigfield::unsafe_evaluate_multiple_multiply_add(const std::vect field_t remainder_prime_limb = field_t::accumulate(prime_limb_accumulator); bb::non_native_multiplication_witnesses witnesses{ - { - left[0].binary_basis_limbs[0].element.get_normalized_witness_index(), - left[0].binary_basis_limbs[1].element.get_normalized_witness_index(), - left[0].binary_basis_limbs[2].element.get_normalized_witness_index(), - left[0].binary_basis_limbs[3].element.get_normalized_witness_index(), - }, - { - right[0].binary_basis_limbs[0].element.get_normalized_witness_index(), - right[0].binary_basis_limbs[1].element.get_normalized_witness_index(), - right[0].binary_basis_limbs[2].element.get_normalized_witness_index(), - right[0].binary_basis_limbs[3].element.get_normalized_witness_index(), - }, - { - quotient.binary_basis_limbs[0].element.get_normalized_witness_index(), - quotient.binary_basis_limbs[1].element.get_normalized_witness_index(), - quotient.binary_basis_limbs[2].element.get_normalized_witness_index(), - quotient.binary_basis_limbs[3].element.get_normalized_witness_index(), - }, + left[0].get_binary_basis_limb_witness_indices(), + right[0].get_binary_basis_limb_witness_indices(), + quotient.get_binary_basis_limb_witness_indices(), { remainder_limbs[0].get_normalized_witness_index(), remainder_limbs[1].get_normalized_witness_index(), From 697cb218b1f9b92faea710638cde84d80cd4a400 Mon Sep 17 00:00:00 2001 From: suyash67 Date: Mon, 9 Jun 2025 13:18:03 +0200 Subject: [PATCH 08/34] remove TODO: this one suggests auditing the `evaluate_linear_identity` fn in field class. this fn was audited in sergei's internal field audit. further this todo is duplicated in field_conversion.cpp and doesn't seem relevant here anymore. hence removing. --- .../barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp | 1 - 1 file changed, 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp index 5eb34e7ed4a0..2a85291ea142 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp @@ -40,7 +40,6 @@ bigfield::bigfield(Builder* parent_context, const uint256_t& value) ASSERT(value < modulus); } -// TODO(https://github.com/AztecProtocol/barretenberg/issues/850): audit the evaluate_linear_identity function template bigfield::bigfield(const field_t& low_bits_in, const field_t& high_bits_in, From ad1dc830b3baff52957e79c318b0e684f739ef86 Mon Sep 17 00:00:00 2001 From: suyash67 Date: Mon, 9 Jun 2025 13:29:43 +0200 Subject: [PATCH 09/34] use is_constant. --- .../src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp index f46eaddd7433..0aabe08e7f1a 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp @@ -589,7 +589,7 @@ template class bigfield { * * TODO(https://github.com/AztecProtocol/aztec-packages/issues/14662): should we check if all limbs are constants? */ - bool is_constant() const { return prime_basis_limb.witness_index == IS_CONSTANT; } + bool is_constant() const { return prime_basis_limb.is_constant(); } /** * @brief Inverting function with the assumption that the bigfield element we are calling invert on is not zero. From b9e816ac374a7766db822fbece4a7d7d9d1a4e5f Mon Sep 17 00:00:00 2001 From: suyash67 Date: Mon, 9 Jun 2025 13:36:58 +0200 Subject: [PATCH 10/34] make sure all limbs is_constant is the same in is_constant fn. fix err. --- .../stdlib/primitives/bigfield/bigfield.hpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp index 0aabe08e7f1a..e85dca3ec153 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp @@ -9,6 +9,7 @@ #include "../byte_array/byte_array.hpp" #include "../circuit_builders/circuit_builders_fwd.hpp" #include "../field/field.hpp" +#include "barretenberg/common/assert.hpp" #include "barretenberg/ecc/curves/bn254/fq.hpp" #include "barretenberg/ecc/curves/bn254/fr.hpp" #include "barretenberg/numeric/uint256/uint256.hpp" @@ -589,7 +590,14 @@ template class bigfield { * * TODO(https://github.com/AztecProtocol/aztec-packages/issues/14662): should we check if all limbs are constants? */ - bool is_constant() const { return prime_basis_limb.is_constant(); } + bool is_constant() const + { + ASSERT(binary_basis_limbs[0].element.is_constant() == binary_basis_limbs[1].element.is_constant() && + binary_basis_limbs[1].element.is_constant() == binary_basis_limbs[2].element.is_constant() && + binary_basis_limbs[2].element.is_constant() == binary_basis_limbs[3].element.is_constant() && + binary_basis_limbs[3].element.is_constant() == prime_basis_limb.is_constant()); + return prime_basis_limb.is_constant(); + } /** * @brief Inverting function with the assumption that the bigfield element we are calling invert on is not zero. From 20808fd8d8204625e016f91dff8ee34d42255710 Mon Sep 17 00:00:00 2001 From: suyash67 Date: Mon, 9 Jun 2025 16:59:26 +0200 Subject: [PATCH 11/34] remove todo issue#14662 on ap. --- .../src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp index e85dca3ec153..f6bccccc4136 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp @@ -588,7 +588,7 @@ template class bigfield { * * @return true if the bigfield is constant, false otherwise. * - * TODO(https://github.com/AztecProtocol/aztec-packages/issues/14662): should we check if all limbs are constants? + * @details We use assertions to ensure that all limbs are consistent in their constant status. */ bool is_constant() const { From d1fff0bfe6727922530bf85ed48747f4325c8bc8 Mon Sep 17 00:00:00 2001 From: suyash67 Date: Sat, 14 Jun 2025 10:05:31 +0200 Subject: [PATCH 12/34] add todo in ultra circuit builder: decompose_into_default_range. potential bug. --- .../circuit_checker/ultra_circuit_builder.test.cpp | 9 +++++++++ .../stdlib_circuit_builders/ultra_circuit_builder.cpp | 2 ++ 2 files changed, 11 insertions(+) diff --git a/barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_builder.test.cpp b/barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_builder.test.cpp index 3019532f3b81..e7d8de12f863 100644 --- a/barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_builder.test.cpp +++ b/barretenberg/cpp/src/barretenberg/circuit_checker/ultra_circuit_builder.test.cpp @@ -556,6 +556,7 @@ TEST(UltraCircuitBuilder, SortWidgetNeg) TEST(UltraCircuitBuilder, ComposedRangeConstraint) { + // even num bits - not divisible by 3 UltraCircuitBuilder builder = UltraCircuitBuilder(); auto c = fr::random_element(); auto d = uint256_t(c).slice(0, 133); @@ -564,6 +565,14 @@ TEST(UltraCircuitBuilder, ComposedRangeConstraint) builder.create_add_gate({ a_idx, builder.zero_idx, builder.zero_idx, 1, 0, 0, -fr(e) }); builder.decompose_into_default_range(a_idx, 134); + // odd num bits - divisible by 3 + auto c_1 = fr::random_element(); + auto d_1 = uint256_t(c_1).slice(0, 126); + auto e_1 = fr(d_1); + auto a_idx_1 = builder.add_variable(fr(e_1)); + builder.create_add_gate({ a_idx_1, builder.zero_idx, builder.zero_idx, 1, 0, 0, -fr(e_1) }); + builder.decompose_into_default_range(a_idx_1, 127); + bool result = CircuitChecker::check(builder); EXPECT_EQ(result, true); } 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 34acdfdc034b..2b0e9cc1f51a 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 @@ -958,6 +958,8 @@ std::vector UltraCircuitBuilder_::decompose_into_defau 0, }, ((i == num_limb_triples - 1) ? false : true)); + // TODO(https://github.com/AztecProtocol/barretenberg/issues/1450): this is probably creating an unused + // wire/variable in the circuit, in the last iteration of the loop. accumulator_idx = this->add_variable(new_accumulator); accumulator = new_accumulator; } From aaa7fa8c3f970b7eed076bafa1725cac33e8dc72 Mon Sep 17 00:00:00 2001 From: suyash67 Date: Thu, 12 Jun 2025 14:33:55 +0200 Subject: [PATCH 13/34] remove == todo, not necessary. resolves bb#999. --- .../barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp index 2a85291ea142..902aa6f3021c 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp @@ -1750,9 +1750,6 @@ template bool_t bigfield::op is_equal.set_origin_tag(OriginTag(get_origin_tag(), other.get_origin_tag())); bigfield diff = (*this) - other; - - // TODO(https://github.com/AztecProtocol/barretenberg/issues/999): get native values efficiently (i.e. if u512 - // value fits in a u256, subtract off modulus until u256 fits into finite field) native diff_native = native((diff.get_value() % modulus_u512).lo); native inverse_native = is_equal_raw ? 0 : diff_native.invert(); From 60f8c98df90f0af42b988bf8f6466cf6ef0d532f Mon Sep 17 00:00:00 2001 From: suyash67 Date: Fri, 20 Jun 2025 13:26:55 +0000 Subject: [PATCH 14/34] simplification (no logic change) in assert_less_than. --- .../stdlib/primitives/bigfield/bigfield_impl.hpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp index 902aa6f3021c..281927b53af4 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp @@ -1842,8 +1842,7 @@ template void bigfield::assert_is_in_ template void bigfield::assert_less_than(const uint256_t upper_limit) const { // Warning: this assumes we have run circuit construction at least once in debug mode where large non reduced - // constants are allowed via ASSERT - + // constants are NOT allowed via ASSERT if (is_constant()) { ASSERT(get_value() < static_cast(upper_limit)); return; @@ -1861,11 +1860,13 @@ template void bigfield::assert_less_t const uint256_t upper_limit_value_2 = strict_upper_limit.slice(NUM_LIMB_BITS * 2, NUM_LIMB_BITS * 3); const uint256_t upper_limit_value_3 = strict_upper_limit.slice(NUM_LIMB_BITS * 3, NUM_LIMB_BITS * 4); - bool borrow_0_value = value.slice(0, NUM_LIMB_BITS) > upper_limit_value_0; - bool borrow_1_value = - (value.slice(NUM_LIMB_BITS, NUM_LIMB_BITS * 2) + uint256_t(borrow_0_value)) > (upper_limit_value_1); - bool borrow_2_value = - (value.slice(NUM_LIMB_BITS * 2, NUM_LIMB_BITS * 3) + uint256_t(borrow_1_value)) > (upper_limit_value_2); + const uint256_t val_0 = value.slice(0, NUM_LIMB_BITS); + const uint256_t val_1 = value.slice(NUM_LIMB_BITS, NUM_LIMB_BITS * 2); + const uint256_t val_2 = value.slice(NUM_LIMB_BITS * 2, NUM_LIMB_BITS * 3); + + bool borrow_0_value = val_0 > upper_limit_value_0; + bool borrow_1_value = (val_1 + uint256_t(borrow_0_value)) > (upper_limit_value_1); + bool borrow_2_value = (val_2 + uint256_t(borrow_1_value)) > (upper_limit_value_2); field_t upper_limit_0(context, upper_limit_value_0); field_t upper_limit_1(context, upper_limit_value_1); @@ -1874,6 +1875,7 @@ template void bigfield::assert_less_t bool_t borrow_0(witness_t(context, borrow_0_value)); bool_t borrow_1(witness_t(context, borrow_1_value)); bool_t borrow_2(witness_t(context, borrow_2_value)); + // The way we use borrows here ensures that we are checking that upper_limit - binary_basis > 0. // We check that the result in each limb is > 0. // If the modulus part in this limb is smaller, we simply borrow the value from the higher limb. From b1aeba00e9bfff8ffec23de6f1e220f5f3cdb174 Mon Sep 17 00:00:00 2001 From: suyash67 Date: Fri, 13 Jun 2025 15:06:49 +0200 Subject: [PATCH 15/34] resolves ap#14660, we don't allow some limbs to be const and some not. either all are const or all witness. --- .../barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp index 281927b53af4..fa4ce2768939 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp @@ -2008,8 +2008,6 @@ template void bigfield::self_reduce() return; } OriginTag new_tag = get_origin_tag(); - // TODO(https://github.com/AztecProtocol/aztec-packages/issues/14660): handle situation where some limbs are - // constant and others are not constant const auto [quotient_value, remainder_value] = get_value().divmod(target_basis.modulus); bigfield quotient(context); From 1a757864421eb9ed8328f4464389d94d6747b5e1 Mon Sep 17 00:00:00 2001 From: suyash67 Date: Fri, 20 Jun 2025 10:33:22 +0000 Subject: [PATCH 16/34] remove todo from reduction check, not necessary to pursue. resolve ap#14658. --- .../stdlib/primitives/bigfield/bigfield_impl.hpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp index fa4ce2768939..a40c39d3feda 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp @@ -1776,10 +1776,7 @@ template bool_t bigfield::op template void bigfield::reduction_check() const { - - if (is_constant()) { // this seems not a reduction check, but actually computing the reduction - // TODO(https://github.com/AztecProtocol/aztec-packages/issues/14658) THIS IS UGLY WHY CAN'T WE - // JUST DO (*THIS) = REDUCED? + if (is_constant()) { uint256_t reduced_value = (get_value() % modulus_u512).lo; bigfield reduced(context, uint256_t(reduced_value)); // Save tags @@ -1788,11 +1785,14 @@ template void bigfield::reduction_che binary_basis_limbs[2].element.get_origin_tag(), binary_basis_limbs[3].element.get_origin_tag(), prime_basis_limb.get_origin_tag() }); + + // Directly assign to mutable members (avoiding assignment operator) binary_basis_limbs[0] = reduced.binary_basis_limbs[0]; binary_basis_limbs[1] = reduced.binary_basis_limbs[1]; binary_basis_limbs[2] = reduced.binary_basis_limbs[2]; binary_basis_limbs[3] = reduced.binary_basis_limbs[3]; prime_basis_limb = reduced.prime_basis_limb; + // Preserve origin tags (useful in simulator) binary_basis_limbs[0].element.set_origin_tag(origin_tags[0]); binary_basis_limbs[1].element.set_origin_tag(origin_tags[1]); From 3a203e3d22a1d14b75b700f8809f3c8b1568d2e5 Mon Sep 17 00:00:00 2001 From: suyash67 Date: Sun, 22 Jun 2025 14:29:07 +0000 Subject: [PATCH 17/34] use efficient div to calculate constant to add, should not change circuit at all. --- .../primitives/bigfield/bigfield_impl.hpp | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp index a40c39d3feda..76a5bfa24e92 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp @@ -573,11 +573,9 @@ bigfield bigfield::operator-(const bigfield& other) cons * * Start by setting constant_to_add = p **/ - uint512_t constant_to_add = modulus_u512; - // add a large enough multiple of p to not get negative result in subtraction - while (constant_to_add.slice(NUM_LIMB_BITS * 3, NUM_LIMB_BITS * 4).lo <= limb_3_maximum_value) { - constant_to_add += modulus_u512; - } + uint1024_t constant_to_add_factor = + (uint1024_t(limb_3_maximum_value) << (NUM_LIMB_BITS * 3)) / uint1024_t(modulus_u512) + uint1024_t(1); + uint512_t constant_to_add = constant_to_add_factor.lo * modulus_u512; /** * Step 3: Compute offset terms t0, t1, t2, t3 that we add to our result to ensure each limb is positive @@ -586,6 +584,16 @@ bigfield bigfield::operator-(const bigfield& other) cons * t2, t1, t0 are the terms we will ADD to constant_to_add.limb[2], constant_to_add.limb[1], *constant_to_add.limb[0] * + * Borrow propagation table: + * ┌───────┬─────────────────────────────────┬──────────────────────────────────┐ + * │ Limb │ Value received FROM next limb │ Value given TO previous limb │ + * ├───────┼─────────────────────────────────┼──────────────────────────────────┤ + * │ 0 │ 2^limb_0_borrow_shift │ 0 │ + * │ 1 │ 2^limb_1_borrow_shift │ 2^(limb_0_borrow_shift - L) │ + * │ 2 │ 2^limb_2_borrow_shift │ 2^(limb_1_borrow_shift - L) │ + * │ 3 │ 0 │ 2^(limb_2_borrow_shift - L) │ + * └───────┴─────────────────────────────────┴──────────────────────────────────┘ + * * i.e. The net value we add to `constant_to_add` is 0. We must ensure that: * t3 = t0 + (t1 << NUM_LIMB_BITS) + (t2 << NUM_LIMB_BITS * 2) * From f5077034456ae73ae7ce384ff8584828d6f4534d Mon Sep 17 00:00:00 2001 From: suyash67 Date: Sun, 22 Jun 2025 19:06:00 +0000 Subject: [PATCH 18/34] change the dummy gate TODO to a NOTE. --- .../stdlib/primitives/bigfield/bigfield_impl.hpp | 5 +++-- .../stdlib_circuit_builders/ultra_circuit_builder.cpp | 9 ++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp index 76a5bfa24e92..f7e1105db984 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp @@ -169,8 +169,9 @@ bigfield bigfield::create_from_u512_as_witness(Builder* -1, 0 }, true); - // TODO(https://github.com/AztecProtocol/barretenberg/issues/879): dummy necessary for preceeding big add - // gate + // NOTE(https://github.com/AztecProtocol/barretenberg/issues/879): Optimisation opportunity to use a single gate + // (and remove dummy gate). Currently, dummy gate is necessary for preceeding big add gate as these gates fall in + // the arithmetic block. More details on the linked Github issue. ctx->create_dummy_gate( ctx->blocks.arithmetic, ctx->zero_idx, ctx->zero_idx, ctx->zero_idx, limb_0.get_normalized_witness_index()); 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 2b0e9cc1f51a..8059b4d2d66a 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 @@ -1256,11 +1256,10 @@ void UltraCircuitBuilder_::create_sort_constraint_with_edges( check_selector_length_consistency(); } - // dummy gate needed because of sort widget's check of next row - // use this gate to check end condition - // TODO(https://github.com/AztecProtocol/barretenberg/issues/879): This was formerly a single arithmetic gate. A - // dummy gate has been added to allow the previous gate to access the required wire data via shifts, allowing the - // arithmetic gate to occur out of sequence. + // NOTE(https://github.com/AztecProtocol/barretenberg/issues/879): Optimisation opportunity to use a single gate + // (and remove dummy gate). This used to be a single gate before trace sorting based on gate types. The dummy gate + // has been added to allow the previous gate to access the required wire data via shifts, allowing the arithmetic + // gate to occur out of sequence. More details on the linked Github issue. create_dummy_gate(block, variable_index[variable_index.size() - 1], this->zero_idx, this->zero_idx, this->zero_idx); create_add_gate({ variable_index[variable_index.size() - 1], this->zero_idx, this->zero_idx, 1, 0, 0, -end }); } From 86ff6db87181986c7f2729abfb4484b4c2237b04 Mon Sep 17 00:00:00 2001 From: suyash67 Date: Sun, 22 Jun 2025 19:07:21 +0000 Subject: [PATCH 19/34] get rid of old TODO. --- .../barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp index f7e1105db984..9e88f5d2ab6c 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp @@ -894,8 +894,6 @@ bigfield bigfield::div_without_denominator_check(const b * Div method with constraints for denominator!=0. * * Similar to operator/ but numerator can be linear sum of multiple elements - * - * TODO: After we create a mechanism for easy updating of witnesses, create a test with proof check **/ template bigfield bigfield::div_check_denominator_nonzero(const std::vector& numerators, From 7c3a6789f0d1c35d27d9a737ce302319de1533d6 Mon Sep 17 00:00:00 2001 From: suyash67 Date: Sun, 22 Jun 2025 19:18:17 +0000 Subject: [PATCH 20/34] resolved TODO related to redundant multiplication ==> we cannot simplify it. --- .../barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp index 9e88f5d2ab6c..2907264db281 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp @@ -1764,8 +1764,7 @@ template bool_t bigfield::op // We need to manually propagate the origin tag inverse.set_origin_tag(OriginTag(get_origin_tag(), other.get_origin_tag())); - // TODO(https://github.com/AztecProtocol/aztec-packages/issues/14723): investigate whether conditional_assign is - // needed when defining `multiplicand` + bigfield multiplicand = bigfield::conditional_assign(is_equal, one(), inverse); bigfield product = diff * multiplicand; From 8c5b1899ae7e2e1515189787287348c712563c75 Mon Sep 17 00:00:00 2001 From: suyash67 Date: Sun, 22 Jun 2025 19:49:33 +0000 Subject: [PATCH 21/34] closed the issue re assert_equal when `other` is constant. changed it to NOTE. add assertion. --- .../stdlib/primitives/bigfield/bigfield_impl.hpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp index 2907264db281..67b4422c63c1 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp @@ -1918,9 +1918,14 @@ template void bigfield::assert_equal( ASSERT(get_value() == other.get_value()); // We expect constants to be less than the target modulus return; } else if (other.is_constant()) { - // TODO(https://github.com/AztecProtocol/barretenberg/issues/998): Something is fishy here - // evaluate a strict equality - make sure *this is reduced first, or an honest prover - // might not be able to satisfy these constraints. + // NOTE(https://github.com/AztecProtocol/barretenberg/issues/998): This can lead to a situation where + // an honest prover cannot satisfy the constraints, because `this` is not reduced, but `other` is, i.e., + // `this` = kp + r and `other` = r + // where k is a positive integer. In such a case, the prover cannot satisfy the constraints + // because the limb-differences would not be 0 mod r. Therefore, an honest prover needs to make sure that + // `this` is reduced before calling this method. Also `other` should never be greater than the modulus by + // design. As a precaution, we assert that the circuit-constant `other` is less than the modulus. + ASSERT(other.get_value() < modulus_u512); field_t t0 = (binary_basis_limbs[0].element - other.binary_basis_limbs[0].element); field_t t1 = (binary_basis_limbs[1].element - other.binary_basis_limbs[1].element); field_t t2 = (binary_basis_limbs[2].element - other.binary_basis_limbs[2].element); From 9084982824b51d9aab4753c8cfd015b40d5bb428 Mon Sep 17 00:00:00 2001 From: suyash67 Date: Sun, 22 Jun 2025 20:10:33 +0000 Subject: [PATCH 22/34] change TODO in sqr to NOTE. --- .../src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp index f6bccccc4136..3fdd839ada5d 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp @@ -498,7 +498,7 @@ template class bigfield { * @return bigfield * * @details Costs the same as operator* as it just sets a = b. - * TODO(https://github.com/AztecProtocol/aztec-packages/issues/15089): can optimise this further. + * NOTE(https://github.com/AztecProtocol/aztec-packages/issues/15089): Can optimise this further to save a gate. */ bigfield sqr() const; From d9668ddd61ecbe427bbaa61252d154a624cccf31 Mon Sep 17 00:00:00 2001 From: suyash67 Date: Sun, 22 Jun 2025 21:52:12 +0000 Subject: [PATCH 23/34] TODO in pow: avoid squaring for last bit, low hanging fruit 1. --- .../stdlib/primitives/bigfield/bigfield.hpp | 2 +- .../stdlib/primitives/bigfield/bigfield_impl.hpp | 14 +++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp index 3fdd839ada5d..6a4a5845b219 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp @@ -523,7 +523,7 @@ template class bigfield { * * @todo TODO(https://github.com/AztecProtocol/barretenberg/issues/1014) Improve the efficiency of this function. */ - bigfield pow(const size_t exponent) const; + bigfield pow(const uint32_t exponent) const; /** * @brief Compute a * b + ...to_add = c mod p diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp index 67b4422c63c1..4879dd8f96c5 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp @@ -10,6 +10,7 @@ #include "barretenberg/common/zip_view.hpp" #include "barretenberg/numeric/uint256/uint256.hpp" #include "barretenberg/numeric/uintx/uintx.hpp" +#include #include #include "../circuit_builders/circuit_builders.hpp" @@ -1004,10 +1005,9 @@ bigfield bigfield::sqradd(const std::vector& t return remainder; } -template bigfield bigfield::pow(const size_t exponent) const +template bigfield bigfield::pow(const uint32_t exponent) const { // Just return one immediately - if (exponent == 0) { return bigfield(uint256_t(1)); } @@ -1015,7 +1015,7 @@ template bigfield bigfield bigfield bigfield= 2) { + shifted_exponent >>= 1; + + // Only square if there are more bits to process. + // It is important to avoid squaring in the final iteration as it otherwise results in + // unwanted gates and variables in the circuit. + if (shifted_exponent != 0) { running_power = running_power.sqr(); } - shifted_exponent >>= 1; } return accumulator; } From 58afe6f107067033389311be341c011e3e3c865d Mon Sep 17 00:00:00 2001 From: suyash67 Date: Sun, 22 Jun 2025 22:01:40 +0000 Subject: [PATCH 24/34] add is_constant conditional for pow. --- .../primitives/bigfield/bigfield_impl.hpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp index 4879dd8f96c5..31a184ece1ff 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp @@ -1012,6 +1012,24 @@ template bigfield bigfield 0) { + if (shifted_exponent & 1) { + result_val = (uint1024_t(result_val) * uint1024_t(base) % uint1024_t(modulus_u512)).lo; + } + base = (uint1024_t(base) * uint1024_t(base) % uint1024_t(modulus_u512)).lo; + shifted_exponent >>= 1; + } + return bigfield(this->context, uint256_t(result_val.lo)); + } + bool accumulator_initialized = false; bigfield accumulator; bigfield running_power = *this; From 0de65406e39822b253e155b6fed0079c946911cd Mon Sep 17 00:00:00 2001 From: suyash67 Date: Sun, 22 Jun 2025 22:09:11 +0000 Subject: [PATCH 25/34] change TODO to NOTE in pow. --- .../src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp index 6a4a5845b219..167254803a2c 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp @@ -521,7 +521,8 @@ template class bigfield { * * @details Uses the square-and-multiply algorithm to compute a^exponent mod p. * - * @todo TODO(https://github.com/AztecProtocol/barretenberg/issues/1014) Improve the efficiency of this function. + * NOTE(https://github.com/AztecProtocol/barretenberg/issues/1014) Improve the efficiency of this function using + * sliding window method. */ bigfield pow(const uint32_t exponent) const; From 3ea68cf61d0610c3f2f972367ddcc280ce89bf23 Mon Sep 17 00:00:00 2001 From: suyash67 Date: Mon, 23 Jun 2025 09:28:51 +0000 Subject: [PATCH 26/34] remove old plookup tests, no more req. --- .../primitives/bigfield/bigfield.test.cpp | 88 ------------------- 1 file changed, 88 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.test.cpp index d8d644d359e7..0fe56ea16c3a 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.test.cpp @@ -1394,91 +1394,3 @@ TYPED_TEST(stdlib_bigfield, internal_div_bug_regression) // bool result = verifier.verify_proof(proof); // EXPECT_EQ(result, true); // } - -// // PLOOKUP TESTS -// TEST(stdlib_bigfield_plookup, test_mul) -// { -// plonk::UltraPlonkBuilder builder = plonk::UltraPlonkBuilder(); -// size_t num_repetitions = 1; -// for (size_t i = 0; i < num_repetitions; ++i) { -// fq inputs[3]{ fq::random_element(), fq::random_element(), fq::random_element() }; -// fq_ct a( -// witness_ct(&builder, bb::fr(uint256_t(inputs[0]).slice(0, -// fq_ct::NUM_LIMB_BITS * 2))), witness_ct(&builder, -// fr( -// uint256_t(inputs[0]).slice(fq_ct::NUM_LIMB_BITS * 2, fq_ct::NUM_LIMB_BITS * -// 4)))); -// fq_ct b( -// witness_ct(&builder, bb::fr(uint256_t(inputs[1]).slice(0, -// fq_ct::NUM_LIMB_BITS * 2))), witness_ct(&builder, -// fr( -// uint256_t(inputs[1]).slice(fq_ct::NUM_LIMB_BITS * 2, fq_ct::NUM_LIMB_BITS * -// 4)))); -// std::cerr << "starting mul" << std::endl; -// uint64_t before = builder.get_estimated_num_finalized_gates(); -// fq_ct c = a * b; -// uint64_t after = builder.get_estimated_num_finalized_gates(); -// if (i == num_repetitions - 1) { -// std::cerr << "num gates per mul = " << after - before << std::endl; -// } - -// fq expected = (inputs[0] * inputs[1]); -// expected = expected.from_montgomery_form(); -// uint512_t result = c.get_value(); - -// EXPECT_EQ(result.lo.data[0], expected.data[0]); -// EXPECT_EQ(result.lo.data[1], expected.data[1]); -// EXPECT_EQ(result.lo.data[2], expected.data[2]); -// EXPECT_EQ(result.lo.data[3], expected.data[3]); -// EXPECT_EQ(result.hi.data[0], 0ULL); -// EXPECT_EQ(result.hi.data[1], 0ULL); -// EXPECT_EQ(result.hi.data[2], 0ULL); -// EXPECT_EQ(result.hi.data[3], 0ULL); -// } -// builder.process_range_lists(); -// plonk::PlookupProver prover = composer.create_prover(); -// plonk::PlookupVerifier verifier = composer.create_verifier(); -// plonk::proof proof = prover.construct_proof(); -// bool result = verifier.verify_proof(proof); -// EXPECT_EQ(result, true); -// } - -// TEST(stdlib_bigfield_plookup, test_sqr) -// { -// plonk::UltraPlonkBuilder builder = plonk::UltraPlonkBuilder(); -// size_t num_repetitions = 10; -// for (size_t i = 0; i < num_repetitions; ++i) { -// fq inputs[3]{ fq::random_element(), fq::random_element(), fq::random_element() }; -// fq_ct a( -// witness_ct(&builder, bb::fr(uint256_t(inputs[0]).slice(0, -// fq_ct::NUM_LIMB_BITS * 2))), witness_ct(&builder, -// fr( -// uint256_t(inputs[0]).slice(fq_ct::NUM_LIMB_BITS * 2, fq_ct::NUM_LIMB_BITS * -// 4)))); -// uint64_t before = builder.get_estimated_num_finalized_gates(); -// fq_ct c = a.sqr(); -// uint64_t after = builder.get_estimated_num_finalized_gates(); -// if (i == num_repetitions - 1) { -// std::cerr << "num gates per sqr = " << after - before << std::endl; -// } - -// fq expected = (inputs[0].sqr()); -// expected = expected.from_montgomery_form(); -// uint512_t result = c.get_value(); - -// EXPECT_EQ(result.lo.data[0], expected.data[0]); -// EXPECT_EQ(result.lo.data[1], expected.data[1]); -// EXPECT_EQ(result.lo.data[2], expected.data[2]); -// EXPECT_EQ(result.lo.data[3], expected.data[3]); -// EXPECT_EQ(result.hi.data[0], 0ULL); -// EXPECT_EQ(result.hi.data[1], 0ULL); -// EXPECT_EQ(result.hi.data[2], 0ULL); -// EXPECT_EQ(result.hi.data[3], 0ULL); -// } -// composer.process_range_lists(); -// plonk::PlookupProver prover = composer.create_prover(); -// plonk::PlookupVerifier verifier = composer.create_verifier(); -// plonk::proof proof = prover.construct_proof(); -// bool result = verifier.verify_proof(proof); -// EXPECT_EQ(result, true); -// } From dececd502d97a17ea4da3df5875ae18ac732ad6b Mon Sep 17 00:00:00 2001 From: suyash67 Date: Mon, 23 Jun 2025 09:33:41 +0000 Subject: [PATCH 27/34] remove commented out lines in tests: cleanup. --- .../primitives/bigfield/bigfield.test.cpp | 41 ------------------- 1 file changed, 41 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.test.cpp index 0fe56ea16c3a..7bbb3e77f8cf 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.test.cpp @@ -163,10 +163,6 @@ template class stdlib_bigfield : public testing::Test { std::cerr << "num gates per mul = " << after - before << std::endl; benchmark_info(Builder::NAME_STRING, "Bigfield", "MUL", "Gate Count", after - before); } - // uint256_t modulus{ Bn254FqParams::modulus_0, - // Bn254FqParams::modulus_1, - // Bn254FqParams::modulus_2, - // Bn254FqParams::modulus_3 }; fq expected = (inputs[0] * inputs[1]); expected = expected.from_montgomery_form(); @@ -206,10 +202,6 @@ template class stdlib_bigfield : public testing::Test { std::cerr << "num gates per mul = " << after - before << std::endl; benchmark_info(Builder::NAME_STRING, "Bigfield", "SQR", "Gate Count", after - before); } - // uint256_t modulus{ Bn254FqParams::modulus_0, - // Bn254FqParams::modulus_1, - // Bn254FqParams::modulus_2, - // Bn254FqParams::modulus_3 }; fq expected = (inputs[0].sqr()); expected = expected.from_montgomery_form(); @@ -257,10 +249,6 @@ template class stdlib_bigfield : public testing::Test { std::cerr << "num gates per mul = " << after - before << std::endl; benchmark_info(Builder::NAME_STRING, "Bigfield", "MADD", "Gate Count", after - before); } - // uint256_t modulus{ Bn254FqParams::modulus_0, - // Bn254FqParams::modulus_1, - // Bn254FqParams::modulus_2, - // Bn254FqParams::modulus_3 }; fq expected = (inputs[0] * inputs[1]) + inputs[2]; expected = expected.from_montgomery_form(); @@ -328,17 +316,6 @@ template class stdlib_bigfield : public testing::Test { std::cerr << "num gates with mult_madd = " << after - before << std::endl; benchmark_info(Builder::NAME_STRING, "Bigfield", "MULT_MADD", "Gate Count", after - before); } - /** - before = builder.get_estimated_num_finalized_gates(); - fq_ct f1(0); - for (size_t j = 0; j < number_of_madds; j++) { - f1 += mul_left[j] * mul_right[j] + to_add[j]; - } - after = builder.get_estimated_num_finalized_gates(); - if (i == num_repetitions - 1) { - std::cerr << "num gates with regular multiply_add = " << after - before << std::endl; - } - **/ expected = expected.from_montgomery_form(); uint512_t result = f.get_value(); @@ -394,10 +371,6 @@ template class stdlib_bigfield : public testing::Test { if (i == num_repetitions - 1) { std::cerr << "num gates per mul = " << after - before << std::endl; } - // uint256_t modulus{ Bn254FqParams::modulus_0, - // Bn254FqParams::modulus_1, - // Bn254FqParams::modulus_2, - // Bn254FqParams::modulus_3 }; fq expected = (inputs[0] * inputs[1]) + (inputs[2] * inputs[3]) + inputs[4]; expected = expected.from_montgomery_form(); @@ -443,10 +416,6 @@ template class stdlib_bigfield : public testing::Test { std::cout << "num gates per div = " << after - before << std::endl; benchmark_info(Builder::NAME_STRING, "Bigfield", "DIV", "Gate Count", after - before); } - // uint256_t modulus{ Bn254FqParams::modulus_0, - // Bn254FqParams::modulus_1, - // Bn254FqParams::modulus_2, - // Bn254FqParams::modulus_3 }; fq expected = (inputs[0] / inputs[1]); expected = expected.reduce_once().reduce_once(); @@ -489,10 +458,6 @@ template class stdlib_bigfield : public testing::Test { d.set_origin_tag(next_challenge_tag); fq_ct e = (a + b) / (c + d); EXPECT_EQ(e.get_origin_tag(), first_second_third_merged_tag); - // uint256_t modulus{ Bn254FqParams::modulus_0, - // Bn254FqParams::modulus_1, - // Bn254FqParams::modulus_2, - // Bn254FqParams::modulus_3 }; fq expected = (inputs[0] + inputs[1]) / (inputs[2] + inputs[3]); expected = expected.reduce_once().reduce_once(); @@ -686,18 +651,12 @@ template class stdlib_bigfield : public testing::Test { fq_ct a(witness_ct(&builder, fr(uint256_t(inputs[0]).slice(0, fq_ct::NUM_LIMB_BITS * 2))), witness_ct(&builder, fr(uint256_t(inputs[0]).slice(fq_ct::NUM_LIMB_BITS * 2, fq_ct::NUM_LIMB_BITS * 4)))); - // fq_ct b(witness_ct(&builder, - // fr(uint256_t(inputs[1]).slice(0, fq_ct::NUM_LIMB_BITS * 2))), - // witness_ct(&builder, - // fr(uint256_t(inputs[1]).slice(fq_ct::NUM_LIMB_BITS * 2, - // fq_ct::NUM_LIMB_BITS * 4)))); a.set_origin_tag(submitted_value_origin_tag); typename bn254::bool_ct predicate_a(witness_ct(&builder, true)); predicate_a.set_origin_tag(challenge_origin_tag); - // bool_ct predicate_b(witness_ct(&builder, false)); fq_ct c = a.conditional_negate(predicate_a); From 8678713da8a8f7b80edc3a560a67c0b8ce8d2e90 Mon Sep 17 00:00:00 2001 From: suyash67 Date: Mon, 23 Jun 2025 09:44:34 +0000 Subject: [PATCH 28/34] add div_with_constant test and remove old commented out one. --- .../primitives/bigfield/bigfield.test.cpp | 91 ++++++++++--------- 1 file changed, 48 insertions(+), 43 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.test.cpp index 7bbb3e77f8cf..2f2453c62e52 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.test.cpp @@ -435,6 +435,50 @@ template class stdlib_bigfield : public testing::Test { EXPECT_EQ(result, true); } + static void test_div_with_constant() + { + auto builder = Builder(); + size_t num_repetitions = 10; + for (size_t i = 0; i < num_repetitions; ++i) { + fq inputs[2]{ fq::random_element(), fq::random_element() }; + inputs[0] = inputs[0].reduce_once().reduce_once(); + inputs[1] = inputs[1].reduce_once().reduce_once(); + + // a is witness, b is constant + fq_ct a(witness_ct(&builder, fr(uint256_t(inputs[0]).slice(0, fq_ct::NUM_LIMB_BITS * 2))), + witness_ct(&builder, + fr(uint256_t(inputs[0]).slice(fq_ct::NUM_LIMB_BITS * 2, fq_ct::NUM_LIMB_BITS * 4)))); + fq_ct b(&builder, uint256_t(inputs[1])); + + a.set_origin_tag(submitted_value_origin_tag); + b.set_origin_tag(challenge_origin_tag); + uint64_t before = builder.get_estimated_num_finalized_gates(); + fq_ct c = a / b; + EXPECT_EQ(c.get_origin_tag(), first_two_merged_tag); + uint64_t after = builder.get_estimated_num_finalized_gates(); + if (i == num_repetitions - 1) { + std::cout << "num gates per div against constant = " << after - before << std::endl; + benchmark_info(Builder::NAME_STRING, "Bigfield", "DIV_CONSTANT", "Gate Count", after - before); + } + + fq expected = (inputs[0] / inputs[1]); + expected = expected.reduce_once().reduce_once(); + expected = expected.from_montgomery_form(); + uint512_t result = c.get_value(); + + EXPECT_EQ(result.lo.data[0], expected.data[0]); + EXPECT_EQ(result.lo.data[1], expected.data[1]); + EXPECT_EQ(result.lo.data[2], expected.data[2]); + EXPECT_EQ(result.lo.data[3], expected.data[3]); + EXPECT_EQ(result.hi.data[0], 0ULL); + EXPECT_EQ(result.hi.data[1], 0ULL); + EXPECT_EQ(result.hi.data[2], 0ULL); + EXPECT_EQ(result.hi.data[3], 0ULL); + } + bool result = CircuitChecker::check(builder); + EXPECT_EQ(result, true); + } + static void test_add_and_div() { auto builder = Builder(); @@ -1217,6 +1261,10 @@ TYPED_TEST(stdlib_bigfield, div_without_denominator_check) { TestFixture::test_div(); } +TYPED_TEST(stdlib_bigfield, div_with_constant) +{ + TestFixture::test_div_with_constant(); +} TYPED_TEST(stdlib_bigfield, add_and_div) { TestFixture::test_add_and_div(); @@ -1310,46 +1358,3 @@ TYPED_TEST(stdlib_bigfield, internal_div_bug_regression) TestFixture::test_internal_div_regression2(); TestFixture::test_internal_div_regression3(); } - -// // This test was disabled before the refactor to use TYPED_TEST's/ -// TEST(stdlib_bigfield, DISABLED_test_div_against_constants) -// { -// auto builder = Builder(); -// size_t num_repetitions = 1; -// for (size_t i = 0; i < num_repetitions; ++i) { -// fq inputs[3]{ fq::random_element(), fq::random_element(), fq::random_element() }; -// fq_ct a(witness_ct(&builder, bb::fr(uint256_t(inputs[0]).slice(0, -// fq_ct::NUM_LIMB_BITS * 2))), -// witness_ct( -// &builder, -// fr(uint256_t(inputs[0]).slice(fq_ct::NUM_LIMB_BITS * 2, -// fq_ct::NUM_LIMB_BITS * 4)))); -// fq_ct b1(&builder, uint256_t(inputs[1])); -// fq_ct b2(&builder, uint256_t(inputs[2])); -// fq_ct c = a / (b1 - b2); -// // uint256_t modulus{ bb::Bn254FqParams::modulus_0, -// // Bn254FqParams::modulus_1, -// // Bn254FqParams::modulus_2, -// // Bn254FqParams::modulus_3 }; - -// fq expected = (inputs[0] / (inputs[1] - inputs[2])); -// std::cerr << "denominator = " << inputs[1] - inputs[2] << std::endl; -// std::cerr << "expected = " << expected << std::endl; -// expected = expected.from_montgomery_form(); -// uint512_t result = c.get_value(); - -// EXPECT_EQ(result.lo.data[0], expected.data[0]); -// EXPECT_EQ(result.lo.data[1], expected.data[1]); -// EXPECT_EQ(result.lo.data[2], expected.data[2]); -// EXPECT_EQ(result.lo.data[3], expected.data[3]); -// EXPECT_EQ(result.hi.data[0], 0ULL); -// EXPECT_EQ(result.hi.data[1], 0ULL); -// EXPECT_EQ(result.hi.data[2], 0ULL); -// EXPECT_EQ(result.hi.data[3], 0ULL); -// } -// auto prover = composer.create_prover(); -// auto verifier = composer.create_verifier(); -// plonk::proof proof = prover.construct_proof(); -// bool result = verifier.verify_proof(proof); -// EXPECT_EQ(result, true); -// } From e5d431b6fef2d662e364097fc24410fc501864e5 Mon Sep 17 00:00:00 2001 From: suyash67 Date: Mon, 23 Jun 2025 10:13:42 +0000 Subject: [PATCH 29/34] add ASSERT on numerator size < 16. --- .../barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp index 31a184ece1ff..38a7c802aeb1 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp @@ -801,6 +801,7 @@ bigfield bigfield::internal_div(const std::vector(denominator.get_context(), uint256_t(0)); } From 0a9447686ea8b11e4d4340e5be8d745916d74e0a Mon Sep 17 00:00:00 2001 From: suyash67 Date: Mon, 23 Jun 2025 10:17:23 +0000 Subject: [PATCH 30/34] Assert that inp is constant in lambda fn. --- .../barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp index 38a7c802aeb1..5e137651d044 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp @@ -2475,6 +2475,7 @@ void bigfield::unsafe_evaluate_multiple_multiply_add(const std::vect // expense of 1 extra gate per constant). // const auto convert_constant_to_fixed_witness = [ctx](const bigfield& input) { + ASSERT(input.is_constant()); bigfield output(input); output.prime_basis_limb = field_t::from_witness_index(ctx, ctx->put_constant_variable(input.prime_basis_limb.get_value())); From 283684b01a2c1fe82d6063ca57ec002b486cc25f Mon Sep 17 00:00:00 2001 From: suyash67 Date: Tue, 24 Jun 2025 11:19:11 +0000 Subject: [PATCH 31/34] kesha suggestion: avoid redundant is_constant() calls. --- .../stdlib/primitives/bigfield/bigfield.hpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp index 167254803a2c..736317c9ee14 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.hpp @@ -593,11 +593,14 @@ template class bigfield { */ bool is_constant() const { - ASSERT(binary_basis_limbs[0].element.is_constant() == binary_basis_limbs[1].element.is_constant() && - binary_basis_limbs[1].element.is_constant() == binary_basis_limbs[2].element.is_constant() && - binary_basis_limbs[2].element.is_constant() == binary_basis_limbs[3].element.is_constant() && - binary_basis_limbs[3].element.is_constant() == prime_basis_limb.is_constant()); - return prime_basis_limb.is_constant(); + bool is_limb_0_constant = binary_basis_limbs[0].element.is_constant(); + bool is_limb_1_constant = binary_basis_limbs[1].element.is_constant(); + bool is_limb_2_constant = binary_basis_limbs[2].element.is_constant(); + bool is_limb_3_constant = binary_basis_limbs[3].element.is_constant(); + bool is_prime_limb_constant = prime_basis_limb.is_constant(); + ASSERT(is_limb_0_constant == is_limb_1_constant && is_limb_1_constant == is_limb_2_constant && + is_limb_2_constant == is_limb_3_constant && is_limb_3_constant == is_prime_limb_constant); + return is_prime_limb_constant; } /** From 4dda0ef5eec90e8302e8dabf12478d77788af727 Mon Sep 17 00:00:00 2001 From: suyash67 Date: Tue, 24 Jun 2025 11:44:55 +0000 Subject: [PATCH 32/34] kesha suggestion: add const/witness test case. --- .../primitives/bigfield/bigfield.test.cpp | 33 +++++++++++++++++-- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.test.cpp index 2f2453c62e52..5015cddfa882 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.test.cpp @@ -440,11 +440,12 @@ template class stdlib_bigfield : public testing::Test { auto builder = Builder(); size_t num_repetitions = 10; for (size_t i = 0; i < num_repetitions; ++i) { - fq inputs[2]{ fq::random_element(), fq::random_element() }; + fq inputs[3]{ fq::random_element(), fq::random_element(), fq::random_element() }; inputs[0] = inputs[0].reduce_once().reduce_once(); inputs[1] = inputs[1].reduce_once().reduce_once(); + inputs[2] = inputs[2].reduce_once().reduce_once(); - // a is witness, b is constant + // numerator is witness, denominator is constant fq_ct a(witness_ct(&builder, fr(uint256_t(inputs[0]).slice(0, fq_ct::NUM_LIMB_BITS * 2))), witness_ct(&builder, fr(uint256_t(inputs[0]).slice(fq_ct::NUM_LIMB_BITS * 2, fq_ct::NUM_LIMB_BITS * 4)))); @@ -457,7 +458,7 @@ template class stdlib_bigfield : public testing::Test { EXPECT_EQ(c.get_origin_tag(), first_two_merged_tag); uint64_t after = builder.get_estimated_num_finalized_gates(); if (i == num_repetitions - 1) { - std::cout << "num gates per div against constant = " << after - before << std::endl; + std::cout << "num gates per div of witness/constant = " << after - before << std::endl; benchmark_info(Builder::NAME_STRING, "Bigfield", "DIV_CONSTANT", "Gate Count", after - before); } @@ -474,6 +475,32 @@ template class stdlib_bigfield : public testing::Test { EXPECT_EQ(result.hi.data[1], 0ULL); EXPECT_EQ(result.hi.data[2], 0ULL); EXPECT_EQ(result.hi.data[3], 0ULL); + + // numerator is constant, denominator is witness + fq_ct e(&builder, uint256_t(inputs[2])); + e.set_origin_tag(challenge_origin_tag); + uint64_t before_e = builder.get_estimated_num_finalized_gates(); + fq_ct d = e / a; + EXPECT_EQ(d.get_origin_tag(), first_two_merged_tag); + uint64_t after_e = builder.get_estimated_num_finalized_gates(); + if (i == num_repetitions - 1) { + std::cout << "num gates per div of constant/witness = " << after_e - before_e << std::endl; + benchmark_info(Builder::NAME_STRING, "Bigfield", "DIV_CONSTANT", "Gate Count", after_e - before_e); + } + + fq expected_e = (inputs[2] / inputs[0]); + expected_e = expected_e.reduce_once().reduce_once(); + expected_e = expected_e.from_montgomery_form(); + uint512_t result_e = d.get_value(); + + EXPECT_EQ(result_e.lo.data[0], expected_e.data[0]); + EXPECT_EQ(result_e.lo.data[1], expected_e.data[1]); + EXPECT_EQ(result_e.lo.data[2], expected_e.data[2]); + EXPECT_EQ(result_e.lo.data[3], expected_e.data[3]); + EXPECT_EQ(result_e.hi.data[0], 0ULL); + EXPECT_EQ(result_e.hi.data[1], 0ULL); + EXPECT_EQ(result_e.hi.data[2], 0ULL); + EXPECT_EQ(result_e.hi.data[3], 0ULL); } bool result = CircuitChecker::check(builder); EXPECT_EQ(result, true); From 3a13040482fe2f847d63023d6ac6b277026753fb Mon Sep 17 00:00:00 2001 From: suyash67 Date: Tue, 24 Jun 2025 13:21:19 +0000 Subject: [PATCH 33/34] kesha suggestion 3: assert that atleast one multiplicand is witness. --- .../primitives/bigfield/bigfield_impl.hpp | 30 +++++++++++++++++-- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp index 5e137651d044..3aee3fcee3e4 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield_impl.hpp @@ -1372,8 +1372,8 @@ bigfield bigfield::mult_madd(const std::vector } } - // Now that we know that there is at least 1 non-constant multiplication, we can start estimating reductions, - // etc + // Now that we know that there is at least 1 non-constant multiplication, we can start estimating reductions. + ASSERT(ctx != nullptr); // Compute the constant term we're adding const auto [_, constant_part_remainder_1024] = (sum_of_constant_products + add_right_constant_sum).divmod(modulus); @@ -2324,11 +2324,15 @@ void bigfield::unsafe_evaluate_multiple_multiply_add(const std::vect ASSERT(input_left.size() == input_right.size() && input_left.size() < 1024); // Sanity checks + bool is_left_constant = true; for (auto& el : input_left) { el.sanity_check(); + is_left_constant &= el.is_constant(); } + bool is_right_constant = true; for (auto& el : input_right) { el.sanity_check(); + is_right_constant &= el.is_constant(); } for (auto& el : to_add) { el.sanity_check(); @@ -2337,13 +2341,33 @@ void bigfield::unsafe_evaluate_multiple_multiply_add(const std::vect for (auto& el : input_remainders) { el.sanity_check(); } + + // We must have at least one left or right multiplicand as witnesses. + ASSERT(!is_left_constant || !is_right_constant); + std::vector remainders(input_remainders); std::vector left(input_left); std::vector right(input_right); bigfield quotient = input_quotient; const size_t num_multiplications = input_left.size(); - Builder* ctx = input_left[0].context ? input_left[0].context : input_right[0].context; + // Fetch the context + Builder* ctx = nullptr; + for (const auto& el : input_left) { + if (el.context) { + ctx = el.context; + break; + } + } + if (ctx == nullptr) { + for (const auto& el : input_right) { + if (el.context) { + ctx = el.context; + break; + } + } + } + ASSERT(ctx != nullptr); const auto get_product_maximum = [](const bigfield& left, const bigfield& right) { uint512_t max_b0_inner = (left.binary_basis_limbs[1].maximum_value * right.binary_basis_limbs[0].maximum_value); From c11a828e2eafb3b87784377071af8ce3454bf56e Mon Sep 17 00:00:00 2001 From: suyash67 Date: Tue, 24 Jun 2025 13:21:47 +0000 Subject: [PATCH 34/34] enable madd test, add mult_madd with constant test. --- .../primitives/bigfield/bigfield.test.cpp | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.test.cpp index 5015cddfa882..9cf7ccc6f157 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.test.cpp @@ -336,6 +336,82 @@ template class stdlib_bigfield : public testing::Test { EXPECT_EQ(result, true); } + static void test_mult_madd_with_constants() + { + auto builder = Builder(); + size_t num_repetitions = 1; + const size_t number_of_madds = 16; + for (size_t i = 0; i < num_repetitions; ++i) { + fq mul_left_values[number_of_madds]; + fq mul_right_values[number_of_madds]; + fq to_add_values[number_of_madds]; + + fq expected(0); + + std::vector mul_left; + std::vector mul_right; + std::vector to_add; + mul_left.reserve(number_of_madds); + mul_right.reserve(number_of_madds); + to_add.reserve(number_of_madds); + for (size_t j = 0; j < number_of_madds; j++) { + mul_left_values[j] = fq::random_element(); + mul_right_values[j] = fq::random_element(); + expected += mul_left_values[j] * mul_right_values[j]; + + // Left and right multiplicands are constants + fq_ct left_const = fq_ct(&builder, uint256_t(mul_left_values[j])); + fq_ct right_const = fq_ct(&builder, uint256_t(mul_right_values[j])); + mul_left.emplace_back(left_const); + mul_right.emplace_back(right_const); + mul_left[j].set_origin_tag(submitted_value_origin_tag); + mul_right[j].set_origin_tag(challenge_origin_tag); + + // to_add are witnesses + to_add_values[j] = fq::random_element(); + expected += to_add_values[j]; + to_add.emplace_back( + fq_ct::create_from_u512_as_witness(&builder, uint512_t(uint256_t(to_add_values[j])))); + + // Since this test uses create_from_u512_as_witness, the tags are set to free_witness_tag + // We need to unset them so that we can test the tag propagation logic without interference + to_add[j].unset_free_witness_tag(); + } + + uint64_t before = builder.get_estimated_num_finalized_gates(); + fq_ct f = fq_ct::mult_madd(mul_left, mul_right, to_add); + + // result might not be reduced, so we need to reduce it + f.self_reduce(); + + // mult_madd merges tags + EXPECT_EQ(f.get_origin_tag(), first_two_merged_tag); + uint64_t after = builder.get_estimated_num_finalized_gates(); + if (i == num_repetitions - 1) { + std::cerr << "num gates with mult_madd = " << after - before << std::endl; + benchmark_info(Builder::NAME_STRING, "Bigfield", "MULT_MADD", "Gate Count", after - before); + } + + expected = expected.reduce_once().reduce_once(); + expected = expected.from_montgomery_form(); + uint512_t result = f.get_value(); + + EXPECT_EQ(result.lo.data[0], expected.data[0]); + EXPECT_EQ(result.lo.data[1], expected.data[1]); + EXPECT_EQ(result.lo.data[2], expected.data[2]); + EXPECT_EQ(result.lo.data[3], expected.data[3]); + EXPECT_EQ(result.hi.data[0], 0ULL); + EXPECT_EQ(result.hi.data[1], 0ULL); + EXPECT_EQ(result.hi.data[2], 0ULL); + EXPECT_EQ(result.hi.data[3], 0ULL); + } + if (builder.failed()) { + info("Builder failed with error: ", builder.err()); + }; + bool result = CircuitChecker::check(builder); + EXPECT_EQ(result, true); + } + static void test_dual_madd() { auto builder = Builder(); @@ -1276,10 +1352,18 @@ TYPED_TEST(stdlib_bigfield, sqr) { TestFixture::test_sqr(); } +TYPED_TEST(stdlib_bigfield, madd) +{ + TestFixture::test_madd(); +} TYPED_TEST(stdlib_bigfield, mult_madd) { TestFixture::test_mult_madd(); } +TYPED_TEST(stdlib_bigfield, mult_madd_with_constants) +{ + TestFixture::test_mult_madd_with_constants(); +} TYPED_TEST(stdlib_bigfield, dual_madd) { TestFixture::test_dual_madd();