From bdfb93b5dd23e6297bebe050c46166b944b7d390 Mon Sep 17 00:00:00 2001 From: lucasxia01 Date: Fri, 22 Sep 2023 10:47:53 +0000 Subject: [PATCH 1/8] fixed original minus underflow test modified tests to cover more cases --- .../primitives/safe_uint/safe_uint.test.cpp | 49 ++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.test.cpp index 2662b61b523f..8c9b58bf409b 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.test.cpp @@ -186,6 +186,7 @@ TYPED_TEST(SafeUintTest, TestSubtractMethodUnderflowFails) c = c.subtract(d, suint_ct::MAX_BIT_NUM); FAIL() << "Expected out of range error"; } catch (std::runtime_error const& err) { + EXPECT_TRUE(composer.check_circuit()); EXPECT_EQ(err.what(), std::string("maximum value exceeded in safe_uint subtract")); } catch (...) { FAIL() << "Expected std::runtime_error modulus in safe_uint class"; @@ -208,6 +209,21 @@ TYPED_TEST(SafeUintTest, TestMinusOperator) EXPECT_TRUE(composer.check_circuit()); } +#if !defined(__wasm__) +TYPED_TEST(SafeUintTest, TestMinusOperator1) +{ + STDLIB_TYPE_ALIASES + auto composer = Composer(); + + field_ct a(witness_ct(&composer, 2)); + field_ct b(witness_ct(&composer, 1)); + suint_ct c(a, 2); + suint_ct d(b, 3); + c = c - d; + EXPECT_TRUE(composer.check_circuit()); +} +#endif + #if !defined(__wasm__) TYPED_TEST(SafeUintTest, TestMinusOperatorUnderflowFails) { @@ -218,16 +234,47 @@ TYPED_TEST(SafeUintTest, TestMinusOperatorUnderflowFails) field_ct b(witness_ct(&composer, field_ct::modulus / 2)); suint_ct c(a, 2); suint_ct d(b, suint_ct::MAX_BIT_NUM); + c = c - d; + EXPECT_FALSE(composer.check_circuit()); +} +#endif + +#if !defined(__wasm__) +TYPED_TEST(SafeUintTest, TestMinusOperatorUnderflowFailsEdge1) +{ + STDLIB_TYPE_ALIASES + auto composer = Composer(); + + field_ct a(witness_ct(&composer, 1)); + field_ct b(witness_ct(&composer, 0)); + suint_ct c(a, suint_ct::MAX_BIT_NUM); + suint_ct d(b, suint_ct::MAX_BIT_NUM); try { c = c - d; + FAIL() << "Expected error to be thrown"; } catch (std::runtime_error const& err) { EXPECT_EQ(err.what(), std::string("maximum value exceeded in safe_uint minus operator")); } catch (...) { - FAIL() << "Expected std::runtime_error modulus in safe_uint class"; + FAIL() << "Expected no error, got other error"; } } #endif +#if !defined(__wasm__) +TYPED_TEST(SafeUintTest, TestMinusOperatorUnderflowFails2) +{ + STDLIB_TYPE_ALIASES + auto composer = Composer(); + + field_ct a(witness_ct(&composer, 2)); + field_ct b(witness_ct(&composer, 3)); + suint_ct c(a, 2); + suint_ct d(b, 3); + c = c - d; + EXPECT_FALSE(composer.check_circuit()); +} +#endif + // DIVIDE METHOD TYPED_TEST(SafeUintTest, TestDivideMethod) From baaeec17066fc81add2c08fb5c5d920fb9180ab3 Mon Sep 17 00:00:00 2001 From: lucasxia01 Date: Fri, 22 Sep 2023 12:28:54 +0000 Subject: [PATCH 2/8] added comments to safe_uint_t subtract and operator- --- .../stdlib/primitives/safe_uint/safe_uint.hpp | 42 +++++++++++++++++-- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.hpp index 52b12c19f239..6896819bea54 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.hpp @@ -96,7 +96,15 @@ template class safe_uint_t { explicit operator bool_ct() { return bool_ct(value); } static safe_uint_t from_witness_index(ComposerContext* parent_context, const uint32_t witness_index); - // Subtraction when you have a pre-determined bound on the difference size + /** + * @brief Subtraction when you have a pre-determined bound on the difference size + * @details Same as operator- except with this pre-determined bound `difference_bit_size`. + * + * @param other + * @param difference_bit_size + * @param description + * @return safe_uint_t + */ safe_uint_t subtract(const safe_uint_t& other, const size_t difference_bit_size, std::string const& description = "") const @@ -104,21 +112,47 @@ template class safe_uint_t { ASSERT(difference_bit_size <= MAX_BIT_NUM); ASSERT(!(this->value.is_constant() && other.value.is_constant())); field_ct difference_val = this->value - other.value; + // Creates the range constraint that difference_val is in [0, (1< difference(difference_val, difference_bit_size, format("subtract: ", description)); - // This checks the subtraction is correct for integers without any wraps + // It is possible for underflow to happen and the range constraint to not catch it. + // This is when (a - b) + modulus <= (1<= modulus will ensure that underflow is caught in all + // cases if (difference.current_max + other.current_max > MAX_VALUE) throw_or_abort("maximum value exceeded in safe_uint subtract"); return difference; } + /** + * @brief Does the minus operator on two safe_uint_t objects + * @details First checks the case where both are constants and there is underflow. + * Then, we compute the difference value and create a safe_uint_t from the value + * and with the same max value as `current_max`. + * Constructing the `difference` safe_uint_t will create a range constraint, + * which will catch underflow as long as the difference value does not end up in the range [0, + * current_max]. The only case where it is possible that the difference value can end up in this range, is when + * `current_max` + `other.current_max` exceeds MAX_VALUE (the modulus - 1), so we throw an error in this case. + * + * @param other + * @return safe_uint_t + */ safe_uint_t operator-(const safe_uint_t& other) const { - // We could get a constant underflow + // If both are constants and the operation is an underflow, throw an error + // Constants are part of the circuit so we can check the case when both are constants and throw an error if + // there is underflow. ASSERT(!(this->value.is_constant() && other.value.is_constant() && static_cast(value.get_value()) < static_cast(other.value.get_value()))); field_ct difference_val = this->value - other.value; + + // safe_uint_t constructor creates a range constraint which checks that `difference_val` is within [0, + // current_max]. safe_uint_t difference(difference_val, (size_t)(current_max.get_msb() + 1), "- operator"); - // This checks the subtraction is correct for integers without any wraps + + // If we are subtracting a - b and there is an underflow AND the range constraint fails to catch it, + // this is equivalent to the condition that (a - b) + modulus <= current_max. + // (a-b) is minimized by -other.current_max, so IF current_max + other.current_max >= modulus, + // there may be an underflow that the range constraint fails to catch, so we need to throw an error. if (difference.current_max + other.current_max > MAX_VALUE) throw_or_abort("maximum value exceeded in safe_uint minus operator"); return difference; From bb5d185f7b0e7dcf1353d14f9455401b255cedad Mon Sep 17 00:00:00 2001 From: lucasxia01 Date: Fri, 22 Sep 2023 12:59:13 +0000 Subject: [PATCH 3/8] moving things to source file for readability --- .../stdlib/primitives/safe_uint/safe_uint.cpp | 145 ++++++++++++++++++ .../stdlib/primitives/safe_uint/safe_uint.hpp | 115 +------------- 2 files changed, 150 insertions(+), 110 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.cpp index ce07f16a27f8..3e8aa85c1be4 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.cpp @@ -22,6 +22,151 @@ safe_uint_t safe_uint_t::operator*(const safe_ return safe_uint_t((value * other.value), new_max.lo, IS_UNSAFE); } +/** + * @brief Subtraction when you have a pre-determined bound on the difference size + * @details Same as operator- except with this pre-determined bound `difference_bit_size`. + * + * @tparam ComposerContext + * @param other + * @param difference_bit_size + * @param description + * @return safe_uint_t + */ +template +safe_uint_t safe_uint_t::subtract(const safe_uint_t& other, + const size_t difference_bit_size, + std::string const& description) const +{ + ASSERT(difference_bit_size <= MAX_BIT_NUM); + ASSERT(!(this->value.is_constant() && other.value.is_constant())); + field_ct difference_val = this->value - other.value; + // Creates the range constraint that difference_val is in [0, (1< difference(difference_val, difference_bit_size, format("subtract: ", description)); + // It is possible for underflow to happen and the range constraint to not catch it. + // This is when (a - b) + modulus <= (1<= modulus will ensure that underflow is caught in all + // cases + if (difference.current_max + other.current_max > MAX_VALUE) + throw_or_abort("maximum value exceeded in safe_uint subtract"); + return difference; +} + +/** + * @brief Does the minus operator on two safe_uint_t objects + * @details First checks the case where both are constants and there is underflow. + * Then, we compute the difference value and create a safe_uint_t from the value + * and with the same max value as `current_max`. + * Constructing the `difference` safe_uint_t will create a range constraint, + * which will catch underflow as long as the difference value does not end up in the range [0, + * current_max]. The only case where it is possible that the difference value can end up in this range, is when + * `current_max` + `other.current_max` exceeds MAX_VALUE (the modulus - 1), so we throw an error in this case. + * + * @tparam ComposerContext + * @param other + * @return safe_uint_t + */ +template +safe_uint_t safe_uint_t::operator-(const safe_uint_t& other) const +{ + // If both are constants and the operation is an underflow, throw an error + // Constants are part of the circuit so we can check the case when both are constants and throw an error if + // there is underflow. + ASSERT(!(this->value.is_constant() && other.value.is_constant() && + static_cast(value.get_value()) < static_cast(other.value.get_value()))); + field_ct difference_val = this->value - other.value; + + // safe_uint_t constructor creates a range constraint which checks that `difference_val` is within [0, + // current_max]. + safe_uint_t difference(difference_val, (size_t)(current_max.get_msb() + 1), "- operator"); + + // If we are subtracting a - b and there is an underflow AND the range constraint fails to catch it, + // this is equivalent to the condition that (a - b) + modulus <= current_max. + // (a-b) is minimized by -other.current_max, so IF current_max + other.current_max >= modulus, + // there may be an underflow that the range constraint fails to catch, so we need to throw an error. + if (difference.current_max + other.current_max > MAX_VALUE) + throw_or_abort("maximum value exceeded in safe_uint minus operator"); + return difference; +} + +/** + * @brief division when you have a pre-determined bound on the sizes of the quotient and remainder + * + * @tparam ComposerContext + * @param other + * @param quotient_bit_size + * @param remainder_bit_size + * @param description + * @param get_quotient + * @return safe_uint_t + */ +template +safe_uint_t safe_uint_t::divide( + const safe_uint_t& other, + const size_t quotient_bit_size, + const size_t remainder_bit_size, + std::string const& description, + const std::function(uint256_t, uint256_t)>& get_quotient) const +{ + ASSERT(this->value.is_constant() == false); + ASSERT(quotient_bit_size <= MAX_BIT_NUM); + ASSERT(remainder_bit_size <= MAX_BIT_NUM); + uint256_t val = this->value.get_value(); + auto [quotient_val, remainder_val] = get_quotient(val, (uint256_t)other.value.get_value()); + field_ct quotient_field(witness_t(value.context, quotient_val)); + field_ct remainder_field(witness_t(value.context, remainder_val)); + safe_uint_t quotient( + quotient_field, quotient_bit_size, format("divide method quotient: ", description)); + safe_uint_t remainder( + remainder_field, remainder_bit_size, format("divide method remainder: ", description)); + + // This line implicitly checks we are not overflowing + safe_uint_t int_val = quotient * other + remainder; + + // We constrain divisor - remainder - 1 to be non-negative to ensure that remainder < divisor. + // Define remainder_plus_one to avoid multiple subtractions + const safe_uint_t remainder_plus_one = remainder + 1; + // Subtraction of safe_uint_t's imposes the desired range constraint + other - remainder_plus_one; + + this->assert_equal(int_val, "divide method quotient and/or remainder incorrect"); + + return quotient; +} + +/** + * @brief Potentially less efficient than divide function - bounds remainder and quotient by max of this + * + * @tparam ComposerContext + * @param other + * @return safe_uint_t + */ +template +safe_uint_t safe_uint_t::operator/(const safe_uint_t& other) const +{ + ASSERT(this->value.is_constant() == false); + uint256_t val = this->value.get_value(); + auto [quotient_val, remainder_val] = val.divmod((uint256_t)other.value.get_value()); + field_ct quotient_field(witness_t(value.context, quotient_val)); + field_ct remainder_field(witness_t(value.context, remainder_val)); + safe_uint_t quotient( + quotient_field, (size_t)(current_max.get_msb() + 1), format("/ operator quotient")); + safe_uint_t remainder( + remainder_field, (size_t)(other.current_max.get_msb() + 1), format("/ operator remainder")); + + // This line implicitly checks we are not overflowing + safe_uint_t int_val = quotient * other + remainder; + + // We constrain divisor - remainder - 1 to be non-negative to ensure that remainder < divisor. + // // define remainder_plus_one to avoid multiple subtractions + const safe_uint_t remainder_plus_one = remainder + 1; + // // subtraction of safe_uint_t's imposes the desired range constraint + other - remainder_plus_one; + + this->assert_equal(int_val, "/ operator quotient and/or remainder incorrect"); + + return quotient; +} + template safe_uint_t safe_uint_t::normalize() const { auto norm_value = value.normalize(); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.hpp index 6896819bea54..b0e3c8a877e4 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.hpp @@ -96,67 +96,12 @@ template class safe_uint_t { explicit operator bool_ct() { return bool_ct(value); } static safe_uint_t from_witness_index(ComposerContext* parent_context, const uint32_t witness_index); - /** - * @brief Subtraction when you have a pre-determined bound on the difference size - * @details Same as operator- except with this pre-determined bound `difference_bit_size`. - * - * @param other - * @param difference_bit_size - * @param description - * @return safe_uint_t - */ + // Subtraction when you have a pre-determined bound on the difference size safe_uint_t subtract(const safe_uint_t& other, const size_t difference_bit_size, - std::string const& description = "") const - { - ASSERT(difference_bit_size <= MAX_BIT_NUM); - ASSERT(!(this->value.is_constant() && other.value.is_constant())); - field_ct difference_val = this->value - other.value; - // Creates the range constraint that difference_val is in [0, (1< difference(difference_val, difference_bit_size, format("subtract: ", description)); - // It is possible for underflow to happen and the range constraint to not catch it. - // This is when (a - b) + modulus <= (1<= modulus will ensure that underflow is caught in all - // cases - if (difference.current_max + other.current_max > MAX_VALUE) - throw_or_abort("maximum value exceeded in safe_uint subtract"); - return difference; - } + std::string const& description = "") const; - /** - * @brief Does the minus operator on two safe_uint_t objects - * @details First checks the case where both are constants and there is underflow. - * Then, we compute the difference value and create a safe_uint_t from the value - * and with the same max value as `current_max`. - * Constructing the `difference` safe_uint_t will create a range constraint, - * which will catch underflow as long as the difference value does not end up in the range [0, - * current_max]. The only case where it is possible that the difference value can end up in this range, is when - * `current_max` + `other.current_max` exceeds MAX_VALUE (the modulus - 1), so we throw an error in this case. - * - * @param other - * @return safe_uint_t - */ - safe_uint_t operator-(const safe_uint_t& other) const - { - // If both are constants and the operation is an underflow, throw an error - // Constants are part of the circuit so we can check the case when both are constants and throw an error if - // there is underflow. - ASSERT(!(this->value.is_constant() && other.value.is_constant() && - static_cast(value.get_value()) < static_cast(other.value.get_value()))); - field_ct difference_val = this->value - other.value; - - // safe_uint_t constructor creates a range constraint which checks that `difference_val` is within [0, - // current_max]. - safe_uint_t difference(difference_val, (size_t)(current_max.get_msb() + 1), "- operator"); - - // If we are subtracting a - b and there is an underflow AND the range constraint fails to catch it, - // this is equivalent to the condition that (a - b) + modulus <= current_max. - // (a-b) is minimized by -other.current_max, so IF current_max + other.current_max >= modulus, - // there may be an underflow that the range constraint fails to catch, so we need to throw an error. - if (difference.current_max + other.current_max > MAX_VALUE) - throw_or_abort("maximum value exceeded in safe_uint minus operator"); - return difference; - } + safe_uint_t operator-(const safe_uint_t& other) const; // division when you have a pre-determined bound on the sizes of the quotient and remainder safe_uint_t divide( @@ -167,60 +112,10 @@ template class safe_uint_t { const std::function(uint256_t, uint256_t)>& get_quotient = [](uint256_t val, uint256_t divisor) { return std::make_pair((uint256_t)(val / (uint256_t)divisor), (uint256_t)(val % (uint256_t)divisor)); - }) - { - ASSERT(this->value.is_constant() == false); - ASSERT(quotient_bit_size <= MAX_BIT_NUM); - ASSERT(remainder_bit_size <= MAX_BIT_NUM); - uint256_t val = this->value.get_value(); - auto [quotient_val, remainder_val] = get_quotient(val, (uint256_t)other.value.get_value()); - field_ct quotient_field(witness_t(value.context, quotient_val)); - field_ct remainder_field(witness_t(value.context, remainder_val)); - safe_uint_t quotient( - quotient_field, quotient_bit_size, format("divide method quotient: ", description)); - safe_uint_t remainder( - remainder_field, remainder_bit_size, format("divide method remainder: ", description)); - - // This line implicitly checks we are not overflowing - safe_uint_t int_val = quotient * other + remainder; - - // We constrain divisor - remainder - 1 to be non-negative to ensure that remainder < divisor. - // Define remainder_plus_one to avoid multiple subtractions - const safe_uint_t remainder_plus_one = remainder + 1; - // Subtraction of safe_uint_t's imposes the desired range constraint - other - remainder_plus_one; - - this->assert_equal(int_val, "divide method quotient and/or remainder incorrect"); - - return quotient; - } + }) const; // Potentially less efficient than divide function - bounds remainder and quotient by max of this - safe_uint_t operator/(const safe_uint_t& other) const - { - ASSERT(this->value.is_constant() == false); - uint256_t val = this->value.get_value(); - auto [quotient_val, remainder_val] = val.divmod((uint256_t)other.value.get_value()); - field_ct quotient_field(witness_t(value.context, quotient_val)); - field_ct remainder_field(witness_t(value.context, remainder_val)); - safe_uint_t quotient( - quotient_field, (size_t)(current_max.get_msb() + 1), format("/ operator quotient")); - safe_uint_t remainder( - remainder_field, (size_t)(other.current_max.get_msb() + 1), format("/ operator remainder")); - - // This line implicitly checks we are not overflowing - safe_uint_t int_val = quotient * other + remainder; - - // We constrain divisor - remainder - 1 to be non-negative to ensure that remainder < divisor. - // // define remainder_plus_one to avoid multiple subtractions - const safe_uint_t remainder_plus_one = remainder + 1; - // // subtraction of safe_uint_t's imposes the desired range constraint - other - remainder_plus_one; - - this->assert_equal(int_val, "/ operator quotient and/or remainder incorrect"); - - return quotient; - } + safe_uint_t operator/(const safe_uint_t& other) const; safe_uint_t add_two(const safe_uint_t& add_a, const safe_uint_t& add_b) const { From d6c6690084ae6bcf4c836be136eef3192291348e Mon Sep 17 00:00:00 2001 From: lucasxia01 Date: Fri, 22 Sep 2023 14:12:25 +0000 Subject: [PATCH 4/8] refactoring/documenting multiply/add tests --- .../primitives/safe_uint/safe_uint.test.cpp | 46 ++++++++++--------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.test.cpp index 8c9b58bf409b..4c8365cefd38 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.test.cpp @@ -70,14 +70,17 @@ TYPED_TEST(SafeUintTest, TestMultiplyOperationOutOfRangeFails) // Since max is initally set to (1 << 2) - 1 = 3 (as bit range checks are easier than generic integer bounds), // should allow largest power of 3 smaller than r iterations, which is 159. Hence below we should exceed r, and // expect a throw + field_ct a(witness_ct(&composer, 2)); + suint_ct c(a, 2); + suint_ct d(a, 2); + // should not fail on 159 iterations, since 3**160 < r < 3**161 + for (auto i = 0; i < 159; i++) { + c = c * d; + } + EXPECT_TRUE(composer.check_circuit()); try { - - field_ct a(witness_ct(&composer, 2)); - suint_ct c(a, 2); - suint_ct d(a, 2); - for (auto i = 0; i < 160; i++) { - c = c * d; - } + // should throw an overflow error on the 160th iteration + c = c * d; FAIL() << "Expected out of range error"; } catch (std::runtime_error const& err) { EXPECT_EQ(err.what(), std::string("exceeded modulus in safe_uint class")); @@ -97,19 +100,16 @@ TYPED_TEST(SafeUintTest, TestMultiplyOperationOnConstantsOutOfRangeFails) suint_ct c(a, 2); suint_ct d(fr(2)); + // should not fail on 252 iterations for (auto i = 0; i < 252; i++) { c = c * d; } + EXPECT_TRUE(composer.check_circuit()); // Below we should exceed r, and expect a throw try { - - field_ct a(witness_ct(&composer, 2)); - suint_ct c(a, 2); - suint_ct d(fr(2)); - for (auto i = 0; i < 253; i++) { - c = c * d; - } + // should fail on the 253rd iteration + c = c * d; FAIL() << "Expected out of range error"; } catch (std::runtime_error const& err) { EXPECT_EQ(err.what(), std::string("exceeded modulus in safe_uint class")); @@ -124,15 +124,17 @@ TYPED_TEST(SafeUintTest, TestAddOperationOutOfRangeFails) STDLIB_TYPE_ALIASES auto composer = Composer(); // Here we test the addition operator also causes a throw when exceeding r + field_ct a(witness_ct(&composer, 2)); + suint_ct c(a, 2); + suint_ct d(a, 2); + // should not fail on the initial setup + for (auto i = 0; i < 159; i++) { + c = c * d; + } + EXPECT_TRUE(composer.check_circuit()); try { - - field_ct a(witness_ct(&composer, 2)); - suint_ct c(a, 2); - suint_ct d(a, 2); - for (auto i = 0; i < 159; i++) { - c = c * d; - } - c = c + c + c; + // should fail when we add and exceed the modulus + c = c + c; FAIL() << "Expected out of range error"; } catch (std::runtime_error const& err) { EXPECT_EQ(err.what(), std::string("exceeded modulus in safe_uint class")); From cdb7be7b5d4b2d47f3145915e5c9bada735e21f2 Mon Sep 17 00:00:00 2001 From: lucasxia01 Date: Fri, 22 Sep 2023 16:08:24 +0000 Subject: [PATCH 5/8] new tests, added comments to tests --- .../stdlib/primitives/safe_uint/safe_uint.cpp | 6 +- .../primitives/safe_uint/safe_uint.test.cpp | 142 +++++++++++++++--- 2 files changed, 124 insertions(+), 24 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.cpp index 3e8aa85c1be4..447aee44ce10 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.cpp @@ -58,8 +58,8 @@ safe_uint_t safe_uint_t::subtract(const safe_u * and with the same max value as `current_max`. * Constructing the `difference` safe_uint_t will create a range constraint, * which will catch underflow as long as the difference value does not end up in the range [0, - * current_max]. The only case where it is possible that the difference value can end up in this range, is when - * `current_max` + `other.current_max` exceeds MAX_VALUE (the modulus - 1), so we throw an error in this case. + * current_max]. The only case where it is possible that the difference value can end up in this range, is when + * `current_max` + `other.current_max` exceeds MAX_VALUE (the modulus - 1), so we throw an error in this case. * * @tparam ComposerContext * @param other @@ -83,6 +83,8 @@ safe_uint_t safe_uint_t::operator-(const safe_ // this is equivalent to the condition that (a - b) + modulus <= current_max. // (a-b) is minimized by -other.current_max, so IF current_max + other.current_max >= modulus, // there may be an underflow that the range constraint fails to catch, so we need to throw an error. + // Note that we will throw an error even if the operation is not an underflow depending on the witnesses + // but we cannot distinguish it from a case that underflows, so we must throw an error. if (difference.current_max + other.current_max > MAX_VALUE) throw_or_abort("maximum value exceeded in safe_uint minus operator"); return difference; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.test.cpp index 4c8365cefd38..67f7aa0aa321 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.test.cpp @@ -62,6 +62,9 @@ TYPED_TEST(SafeUintTest, TestConstructorWithValueInRange) // * OPERATOR +/** + * @brief Test that we overflow correctly on the border of 3**160 and 3**161. + */ #if !defined(__wasm__) TYPED_TEST(SafeUintTest, TestMultiplyOperationOutOfRangeFails) { @@ -83,12 +86,16 @@ TYPED_TEST(SafeUintTest, TestMultiplyOperationOutOfRangeFails) c = c * d; FAIL() << "Expected out of range error"; } catch (std::runtime_error const& err) { + EXPECT_TRUE(composer.check_circuit()); // no failing constraints should be created from multiply EXPECT_EQ(err.what(), std::string("exceeded modulus in safe_uint class")); } catch (...) { FAIL() << "Expected std::runtime_error modulus in safe_uint class"; } } +/** + * @brief Test that we correctly overflow multiplying by a constant on the border of 2**253 and 2**254. + */ TYPED_TEST(SafeUintTest, TestMultiplyOperationOnConstantsOutOfRangeFails) { STDLIB_TYPE_ALIASES @@ -112,6 +119,7 @@ TYPED_TEST(SafeUintTest, TestMultiplyOperationOnConstantsOutOfRangeFails) c = c * d; FAIL() << "Expected out of range error"; } catch (std::runtime_error const& err) { + EXPECT_TRUE(composer.check_circuit()); // no failing constraint from multiply EXPECT_EQ(err.what(), std::string("exceeded modulus in safe_uint class")); } catch (...) { FAIL() << "Expected std::runtime_error modulus in safe_uint class"; @@ -119,6 +127,9 @@ TYPED_TEST(SafeUintTest, TestMultiplyOperationOnConstantsOutOfRangeFails) } // + OPERATOR +/** + * @brief Test that we correctly overflow on addition on the border of 3**160 and 2 * 3**160. + */ TYPED_TEST(SafeUintTest, TestAddOperationOutOfRangeFails) { STDLIB_TYPE_ALIASES @@ -137,14 +148,19 @@ TYPED_TEST(SafeUintTest, TestAddOperationOutOfRangeFails) c = c + c; FAIL() << "Expected out of range error"; } catch (std::runtime_error const& err) { + EXPECT_TRUE(composer.check_circuit()); // no failing constraints from add or multiply EXPECT_EQ(err.what(), std::string("exceeded modulus in safe_uint class")); } catch (...) { FAIL() << "Expected std::runtime_error modulus in safe_uint class"; } } #endif + // SUBTRACT METHOD +/** + * @brief Test that we can subtract without underflow successfully. + */ TYPED_TEST(SafeUintTest, TestSubtractMethod) { STDLIB_TYPE_ALIASES @@ -154,11 +170,15 @@ TYPED_TEST(SafeUintTest, TestSubtractMethod) field_ct b(witness_ct(&composer, 9)); suint_ct c(a, 2); suint_ct d(b, 4); - c = d.subtract(c, 3); + c = d.subtract(c, 3); // result is 7, which fits in 3 bits and does not fail the range constraint EXPECT_TRUE(composer.check_circuit()); } +/** + * @brief Test that range constraint fails if the value exceeds the bit limit. + * @details difference is 7, which exceeds 2 bits, and causes the circuit to fail. + */ TYPED_TEST(SafeUintTest, TestSubtractMethodMinuedGtLhsFails) { STDLIB_TYPE_ALIASES @@ -174,8 +194,12 @@ TYPED_TEST(SafeUintTest, TestSubtractMethodMinuedGtLhsFails) EXPECT_FALSE(composer.check_circuit()); } +/** + * @brief Test that underflow is caught in the special case. + * @details Should fail because difference.current_max + other.current_max exceeds the MAX_VALUE. + */ #if !defined(__wasm__) -TYPED_TEST(SafeUintTest, TestSubtractMethodUnderflowFails) +TYPED_TEST(SafeUintTest, TestSubtractMethodUnderflowFailsSpecial) { STDLIB_TYPE_ALIASES auto composer = Composer(); @@ -195,22 +219,48 @@ TYPED_TEST(SafeUintTest, TestSubtractMethodUnderflowFails) } } #endif + +/** + * @brief Test that underflow is caught in general case. + * @details General case refers to when difference.current_max + other.current_max does not exceed MAX_VALUE. + */ +#if !defined(__wasm__) +TYPED_TEST(SafeUintTest, TestSubtractMethodUnderflowFailsGeneral) +{ + STDLIB_TYPE_ALIASES + auto composer = Composer(); + + field_ct a(witness_ct(&composer, 0)); + field_ct b(witness_ct(&composer, 1)); + suint_ct c(a, 0); + suint_ct d(b, 1); + c = c.subtract(d, suint_ct::MAX_BIT_NUM); + EXPECT_FALSE(composer.check_circuit()); +} +#endif + // - OPERATOR -TYPED_TEST(SafeUintTest, TestMinusOperator) +/** + * @brief Test that valid minus operation works. + */ +TYPED_TEST(SafeUintTest, TestMinusOperator0) { STDLIB_TYPE_ALIASES auto composer = Composer(); - field_ct a(witness_ct(&composer, 2)); - field_ct b(witness_ct(&composer, 9)); - suint_ct c(a, 2); - suint_ct d(b, 4); - c = d - c; + field_ct a(witness_ct(&composer, 9)); + field_ct b(witness_ct(&composer, 2)); + suint_ct c(a, 4); + suint_ct d(b, 2); + c = c - d; // 9 - 2 = 7 should not underflow EXPECT_TRUE(composer.check_circuit()); } +/** + * @brief Test that valid minus operation works on 0. + */ #if !defined(__wasm__) TYPED_TEST(SafeUintTest, TestMinusOperator1) { @@ -218,16 +268,20 @@ TYPED_TEST(SafeUintTest, TestMinusOperator1) auto composer = Composer(); field_ct a(witness_ct(&composer, 2)); - field_ct b(witness_ct(&composer, 1)); + field_ct b(witness_ct(&composer, 2)); suint_ct c(a, 2); suint_ct d(b, 3); - c = c - d; + c = c - d; // 2 - 2 = 0 should not overflow, even if d has more bits than c. EXPECT_TRUE(composer.check_circuit()); } #endif +/** + * @brief Test that checks that minus operator underflow is caught in the general case. + * @details General case means that the special case does not happen. + */ #if !defined(__wasm__) -TYPED_TEST(SafeUintTest, TestMinusOperatorUnderflowFails) +TYPED_TEST(SafeUintTest, TestMinusOperatorUnderflowFailsGeneral0) { STDLIB_TYPE_ALIASES auto composer = Composer(); @@ -236,13 +290,38 @@ TYPED_TEST(SafeUintTest, TestMinusOperatorUnderflowFails) field_ct b(witness_ct(&composer, field_ct::modulus / 2)); suint_ct c(a, 2); suint_ct d(b, suint_ct::MAX_BIT_NUM); - c = c - d; + c = c - d; // generates range constraint that the difference is in [0, 3], which it is not with these witness values EXPECT_FALSE(composer.check_circuit()); } #endif +/** + * @brief Test that checks that minus operator underflow is caught in the general case. + * @details Testing -1 is an underflow. + */ +#if !defined(__wasm__) +TYPED_TEST(SafeUintTest, TestMinusOperatorUnderflowFailsGeneral1) +{ + STDLIB_TYPE_ALIASES + auto composer = Composer(); + + field_ct a(witness_ct(&composer, 2)); + field_ct b(witness_ct(&composer, 3)); + suint_ct c(a, 2); + suint_ct d(b, 3); + c = c - d; + EXPECT_FALSE(composer.check_circuit()); // underflow should cause range constraint to fail +} +#endif + +/** + * @brief Test that checks that minus operator underflow is caught from special case. + * @details Special case refers to the check that current_max + other.current_max > MAX_VALUE, which is a potential + * underflow case, that escapes the general check through the range constraint. Throws an error even if it is not an + * underflow in some instantiations of the witness values. + */ #if !defined(__wasm__) -TYPED_TEST(SafeUintTest, TestMinusOperatorUnderflowFailsEdge1) +TYPED_TEST(SafeUintTest, TestMinusOperatorUnderflowFailsSpecial0) { STDLIB_TYPE_ALIASES auto composer = Composer(); @@ -252,28 +331,47 @@ TYPED_TEST(SafeUintTest, TestMinusOperatorUnderflowFailsEdge1) suint_ct c(a, suint_ct::MAX_BIT_NUM); suint_ct d(b, suint_ct::MAX_BIT_NUM); try { - c = c - d; + c = c - d; // even though this is not an underflow, we cannot distinguish it from an actual underflow because + // the sum of maxes exceeds MAX_VALUE so we must throw an error FAIL() << "Expected error to be thrown"; } catch (std::runtime_error const& err) { - EXPECT_EQ(err.what(), std::string("maximum value exceeded in safe_uint minus operator")); + EXPECT_TRUE(composer.check_circuit()); // no incorrect constraints + EXPECT_EQ(err.what(), + std::string("maximum value exceeded in safe_uint minus operator")); // possible underflow is detected + // with check on maxes } catch (...) { FAIL() << "Expected no error, got other error"; } } #endif +/** + * @brief Test that checks that minus operator underflow is caught from special case. + * @details Special case refers to the check that current_max + other.current_max > MAX_VALUE, which is a potential + * underflow case, that escapes the general check through the range constraint. Also, underflow can actually be detected + * from range constraint. + */ #if !defined(__wasm__) -TYPED_TEST(SafeUintTest, TestMinusOperatorUnderflowFails2) +TYPED_TEST(SafeUintTest, TestMinusOperatorUnderflowFailsSpecial1) { STDLIB_TYPE_ALIASES auto composer = Composer(); - field_ct a(witness_ct(&composer, 2)); - field_ct b(witness_ct(&composer, 3)); - suint_ct c(a, 2); - suint_ct d(b, 3); - c = c - d; - EXPECT_FALSE(composer.check_circuit()); + field_ct a(witness_ct(&composer, 0)); + field_ct b(witness_ct(&composer, 1)); + suint_ct c(a, suint_ct::MAX_BIT_NUM); + suint_ct d(b, suint_ct::MAX_BIT_NUM); + try { + c = c - d; // underflow and error should be thrown + FAIL() << "Expected error to be thrown"; + } catch (std::runtime_error const& err) { + EXPECT_FALSE(composer.check_circuit()); // underflow causes failing constraint + EXPECT_EQ(err.what(), + std::string("maximum value exceeded in safe_uint minus operator")); // possible underflow is detected + // with check on maxes + } catch (...) { + FAIL() << "Expected no error, got other error"; + } } #endif From 8e3f7328465b138f30cd85e230bdbfa632aa595b Mon Sep 17 00:00:00 2001 From: Lucas Xia Date: Tue, 26 Sep 2023 14:10:07 -0400 Subject: [PATCH 6/8] Apply minor suggestions from code review Co-authored-by: maramihali --- .../stdlib/primitives/safe_uint/safe_uint.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.cpp index 447aee44ce10..9cd7510fdc67 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.cpp @@ -52,12 +52,12 @@ safe_uint_t safe_uint_t::subtract(const safe_u } /** - * @brief Does the minus operator on two safe_uint_t objects - * @details First checks the case where both are constants and there is underflow. - * Then, we compute the difference value and create a safe_uint_t from the value - * and with the same max value as `current_max`. + * @brief Subtraction on two safe_uint_t objects + * @details The function first checks the case when both operands are constants and there is underflow. + * Then, it computes the difference and create a safe_uint_t from its value + * with the same max value as `current_max`. * Constructing the `difference` safe_uint_t will create a range constraint, - * which will catch underflow as long as the difference value does not end up in the range [0, + * which catches underflow as long as the difference value does not end up in the range [0, * current_max]. The only case where it is possible that the difference value can end up in this range, is when * `current_max` + `other.current_max` exceeds MAX_VALUE (the modulus - 1), so we throw an error in this case. * @@ -79,7 +79,8 @@ safe_uint_t safe_uint_t::operator-(const safe_ // current_max]. safe_uint_t difference(difference_val, (size_t)(current_max.get_msb() + 1), "- operator"); - // If we are subtracting a - b and there is an underflow AND the range constraint fails to catch it, + // Call the two operands a and b. If this operations is underflow and the range constraint fails to catch it, + // this means that (a-b) + modulus is IN the range [0, current_max] // this is equivalent to the condition that (a - b) + modulus <= current_max. // (a-b) is minimized by -other.current_max, so IF current_max + other.current_max >= modulus, // there may be an underflow that the range constraint fails to catch, so we need to throw an error. From 06a836dbf95905b2aa09a125fa9a6f10f97f30bf Mon Sep 17 00:00:00 2001 From: lucasxia01 Date: Thu, 28 Sep 2023 18:13:07 +0000 Subject: [PATCH 7/8] renamed tests, modified other comments --- .../stdlib/primitives/safe_uint/safe_uint.cpp | 18 +++--- .../primitives/safe_uint/safe_uint.test.cpp | 59 ++++++++++--------- 2 files changed, 39 insertions(+), 38 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.cpp index ff751fc37cad..3ff394271d9e 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.cpp @@ -66,9 +66,7 @@ safe_uint_t safe_uint_t::subtract(const safe_uint_t& other, */ template safe_uint_t safe_uint_t::operator-(const safe_uint_t& other) const { - // If both are constants and the operation is an underflow, throw an error - // Constants are part of the circuit so we can check the case when both are constants and throw an error if - // there is underflow. + // If both are constants and the operation is an underflow, throw an error since circuit itself underflows ASSERT(!(this->value.is_constant() && other.value.is_constant() && static_cast(value.get_value()) < static_cast(other.value.get_value()))); field_ct difference_val = this->value - other.value; @@ -78,12 +76,14 @@ template safe_uint_t safe_uint_t::operator- safe_uint_t difference(difference_val, (size_t)(current_max.get_msb() + 1), "- operator"); // Call the two operands a and b. If this operations is underflow and the range constraint fails to catch it, - // this means that (a-b) + modulus is IN the range [0, current_max] - // this is equivalent to the condition that (a - b) + modulus <= current_max. - // (a-b) is minimized by -other.current_max, so IF current_max + other.current_max >= modulus, - // there may be an underflow that the range constraint fails to catch, so we need to throw an error. - // Note that we will throw an error even if the operation is not an underflow depending on the witnesses - // but we cannot distinguish it from a case that underflows, so we must throw an error. + // this means that (a-b) + modulus is IN the range [0, a.current_max]. + // This is equivalent to the condition that (a - b) + modulus <= a.current_max. + // IF b.current_max >= modulus - a.current_max, then it is possible for this condition to be true + // because we can let a be 0, and b be b.current_max -> (0 - b.current_max) + modulus <= a.current_max is true. + // IF b.current_max < modulus - a.current_max, it is impossible for underflow to happen, no matter how you set a and + // b. Therefore, we check that b.current_max >= modulus - a.current_max, which is equivalent to + // difference.current_max + other.current_max > MAX_VALUE Note that we will throw an error sometimes even if a-b is + // not an underflow but we cannot distinguish it from a case that underflows, so we must throw an error. if (difference.current_max + other.current_max > MAX_VALUE) throw_or_abort("maximum value exceeded in safe_uint minus operator"); return difference; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.test.cpp index 1872eb9e0028..1b687408e605 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.test.cpp @@ -160,7 +160,7 @@ TYPED_TEST(SafeUintTest, TestAddOperationOutOfRangeFails) /** * @brief Test that we can subtract without underflow successfully. */ -TYPED_TEST(SafeUintTest, TestSubtractMethod) +TYPED_TEST(SafeUintTest, TestSubtract) { STDLIB_TYPE_ALIASES auto builder = Builder(); @@ -178,7 +178,7 @@ TYPED_TEST(SafeUintTest, TestSubtractMethod) * @brief Test that range constraint fails if the value exceeds the bit limit. * @details difference is 7, which exceeds 2 bits, and causes the circuit to fail. */ -TYPED_TEST(SafeUintTest, TestSubtractMethodMinuedGtLhsFails) +TYPED_TEST(SafeUintTest, TestSubtractResultOutOfRange) { STDLIB_TYPE_ALIASES auto builder = Builder(); @@ -193,12 +193,32 @@ TYPED_TEST(SafeUintTest, TestSubtractMethodMinuedGtLhsFails) EXPECT_FALSE(builder.check_circuit()); } +/** + * @brief Test that underflow is caught in general case. + * @details General case refers to when difference.current_max + other.current_max does not exceed MAX_VALUE + * and underflow is caught by range constraint. + */ +#if !defined(__wasm__) +TYPED_TEST(SafeUintTest, TestSubtractUnderflowGeneral) +{ + STDLIB_TYPE_ALIASES + auto builder = Builder(); + + field_ct a(witness_ct(&builder, 0)); + field_ct b(witness_ct(&builder, 1)); + suint_ct c(a, 0); + suint_ct d(b, 1); + c = c.subtract(d, suint_ct::MAX_BIT_NUM); + EXPECT_FALSE(builder.check_circuit()); +} +#endif + /** * @brief Test that underflow is caught in the special case. - * @details Should fail because difference.current_max + other.current_max exceeds the MAX_VALUE. + * @details Should throw an error because difference.current_max + other.current_max exceeds the MAX_VALUE. */ #if !defined(__wasm__) -TYPED_TEST(SafeUintTest, TestSubtractMethodUnderflowFailsSpecial) +TYPED_TEST(SafeUintTest, TestSubtractUnderflowSpecial) { STDLIB_TYPE_ALIASES auto builder = Builder(); @@ -219,31 +239,12 @@ TYPED_TEST(SafeUintTest, TestSubtractMethodUnderflowFailsSpecial) } #endif -/** - * @brief Test that underflow is caught in general case. - * @details General case refers to when difference.current_max + other.current_max does not exceed MAX_VALUE. - */ -#if !defined(__wasm__) -TYPED_TEST(SafeUintTest, TestSubtractMethodUnderflowFailsGeneral) -{ - STDLIB_TYPE_ALIASES - auto builder = Builder(); - - field_ct a(witness_ct(&builder, 0)); - field_ct b(witness_ct(&builder, 1)); - suint_ct c(a, 0); - suint_ct d(b, 1); - c = c.subtract(d, suint_ct::MAX_BIT_NUM); - EXPECT_FALSE(builder.check_circuit()); -} -#endif - // - OPERATOR /** * @brief Test that valid minus operation works. */ -TYPED_TEST(SafeUintTest, TestMinusOperator0) +TYPED_TEST(SafeUintTest, TestMinusOperator1) { STDLIB_TYPE_ALIASES auto builder = Builder(); @@ -261,7 +262,7 @@ TYPED_TEST(SafeUintTest, TestMinusOperator0) * @brief Test that valid minus operation works on 0. */ #if !defined(__wasm__) -TYPED_TEST(SafeUintTest, TestMinusOperator1) +TYPED_TEST(SafeUintTest, TestMinusOperator2) { STDLIB_TYPE_ALIASES auto builder = Builder(); @@ -280,7 +281,7 @@ TYPED_TEST(SafeUintTest, TestMinusOperator1) * @details General case means that the special case does not happen. */ #if !defined(__wasm__) -TYPED_TEST(SafeUintTest, TestMinusOperatorUnderflowFailsGeneral0) +TYPED_TEST(SafeUintTest, TestMinusUnderflowGeneral1) { STDLIB_TYPE_ALIASES auto builder = Builder(); @@ -299,7 +300,7 @@ TYPED_TEST(SafeUintTest, TestMinusOperatorUnderflowFailsGeneral0) * @details Testing -1 is an underflow. */ #if !defined(__wasm__) -TYPED_TEST(SafeUintTest, TestMinusOperatorUnderflowFailsGeneral1) +TYPED_TEST(SafeUintTest, TestMinusUnderflowGeneral2) { STDLIB_TYPE_ALIASES auto builder = Builder(); @@ -320,7 +321,7 @@ TYPED_TEST(SafeUintTest, TestMinusOperatorUnderflowFailsGeneral1) * underflow in some instantiations of the witness values. */ #if !defined(__wasm__) -TYPED_TEST(SafeUintTest, TestMinusOperatorUnderflowFailsSpecial0) +TYPED_TEST(SafeUintTest, TestMinusUnderflowSpecial1) { STDLIB_TYPE_ALIASES auto builder = Builder(); @@ -351,7 +352,7 @@ TYPED_TEST(SafeUintTest, TestMinusOperatorUnderflowFailsSpecial0) * from range constraint. */ #if !defined(__wasm__) -TYPED_TEST(SafeUintTest, TestMinusOperatorUnderflowFailsSpecial1) +TYPED_TEST(SafeUintTest, TestMinusUnderflowSpecial2) { STDLIB_TYPE_ALIASES auto builder = Builder(); From 1999d63f2adb07c562070a719e9785d5531109c3 Mon Sep 17 00:00:00 2001 From: Lucas Xia Date: Fri, 29 Sep 2023 11:43:55 -0400 Subject: [PATCH 8/8] Apply suggestions from code review Minor test renaming Co-authored-by: maramihali --- .../stdlib/primitives/safe_uint/safe_uint.test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.test.cpp index 1b687408e605..cf54c1182f81 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/safe_uint/safe_uint.test.cpp @@ -244,7 +244,7 @@ TYPED_TEST(SafeUintTest, TestSubtractUnderflowSpecial) /** * @brief Test that valid minus operation works. */ -TYPED_TEST(SafeUintTest, TestMinusOperator1) +TYPED_TEST(SafeUintTest, TestMinusOperator) { STDLIB_TYPE_ALIASES auto builder = Builder(); @@ -262,7 +262,7 @@ TYPED_TEST(SafeUintTest, TestMinusOperator1) * @brief Test that valid minus operation works on 0. */ #if !defined(__wasm__) -TYPED_TEST(SafeUintTest, TestMinusOperator2) +TYPED_TEST(SafeUintTest, TestMinusOperatorValidOnZero) { STDLIB_TYPE_ALIASES auto builder = Builder();