From 4ee400d39ba09ee80f245ae6bd50eff8da1f1396 Mon Sep 17 00:00:00 2001 From: Suyash Bagad Date: Thu, 23 Mar 2023 20:46:24 +0000 Subject: [PATCH] Address Luke's Comments on `aztec3 -> master` (#263) * Add must_imply tests. * use std honk composer. * Add fail test. * Added a test for `field_t::copy_as_new_witness` * minor change. * add test for `conditional_assign` * Get rid of magic `15` * Comment in pedersen lookup test. * Remove unused/unnecessary condition. * Fix postfix op in ecc field and add tests. * Added `infinity` test. * Fix fr test. * Add `add_affine_test`. --- .../pedersen_lookup.test.cpp | 67 +++++++-- .../barretenberg/ecc/curves/bn254/fr.test.cpp | 29 ++++ .../barretenberg/ecc/curves/bn254/g1.test.cpp | 22 +++ .../barretenberg/ecc/fields/field_impl.hpp | 5 +- .../barretenberg/ecc/groups/element_impl.hpp | 3 - .../stdlib/primitives/bool/bool.cpp | 41 ++++- .../stdlib/primitives/bool/bool.test.cpp | 142 +++++++++++++++++- .../stdlib/primitives/field/field.test.cpp | 28 ++++ 8 files changed, 314 insertions(+), 23 deletions(-) diff --git a/cpp/src/barretenberg/crypto/pedersen_commitment/pedersen_lookup.test.cpp b/cpp/src/barretenberg/crypto/pedersen_commitment/pedersen_lookup.test.cpp index fc8b47eef6..82d0b4f7ed 100644 --- a/cpp/src/barretenberg/crypto/pedersen_commitment/pedersen_lookup.test.cpp +++ b/cpp/src/barretenberg/crypto/pedersen_commitment/pedersen_lookup.test.cpp @@ -16,7 +16,29 @@ auto compute_expected(const grumpkin::fq exponent, size_t generator_offset) const auto lambda = grumpkin::fr::cube_root_of_unity(); const auto mask = crypto::pedersen_hash::lookup::PEDERSEN_TABLE_SIZE - 1; - for (size_t i = 0; i < 15; ++i) { + /** + * Given an input scalar x, we split it into 9-bit slices: + * x = ( x_28 || x_27 || ... || x_2 || x_1 || x_0 ) + * + * Note that the last slice x_28 is a 2-bit slice. Total = 2 + 9 * 28 = 254 bits. + * + * Algorithm: + * hash = O; + * hash += x_0 * G_0 + x_1 * λ * G_0; + * hash += x_2 * G_1 + x_2 * λ * G_1; + * ... + * ... + * hash += x_26 * G_13 + x_27 * λ * G_13; + * hash += x_27 * G_14; + * + * Our lookup tables stores the following: + * 1 -> (G_0, (λ * G_0)) + * 2 -> (2G_0, 2(λ * G_0)) + * 3 -> (3G_0, 3(λ * G_0)) + * ... + * 512 -> (512G_0, 512(λ * G_0)) + */ + for (size_t i = 0; i < (crypto::pedersen_hash::lookup::NUM_PEDERSEN_TABLES / 2); ++i) { const auto slice_a = static_cast(bits.data[0] & mask) + 1; bits >>= crypto::pedersen_hash::lookup::BITS_PER_TABLE; const auto slice_b = static_cast(bits.data[0] & mask) + 1; @@ -81,7 +103,7 @@ TEST(pedersen_lookup, hash_single) std::array accumulators; - for (size_t i = 0; i < 15; ++i) { + for (size_t i = 0; i < (crypto::pedersen_hash::lookup::NUM_PEDERSEN_TABLES / 2); ++i) { const auto slice_a = static_cast(bits.data[0] & mask) + 1; bits >>= crypto::pedersen_hash::lookup::BITS_PER_TABLE; const auto slice_b = static_cast(bits.data[0] & mask) + 1; @@ -115,7 +137,8 @@ TEST(pedersen_lookup, hash_pair) const fq result(crypto::pedersen_hash::lookup::hash_pair(left, right)); - const affine_element expected(compute_expected(left, 0) + compute_expected(right, 15)); + const affine_element expected(compute_expected(left, 0) + + compute_expected(right, (crypto::pedersen_hash::lookup::NUM_PEDERSEN_TABLES / 2))); EXPECT_EQ(result, expected.x); } @@ -136,11 +159,16 @@ TEST(pedersen_lookup, merkle_damgard_compress) fq intermediate = (grumpkin::g1::affine_one * fr(iv + 1)).x; for (size_t i = 0; i < m; i++) { - intermediate = affine_element(compute_expected(intermediate, 0) + compute_expected(inputs[i], 15)).x; + intermediate = + affine_element(compute_expected(intermediate, 0) + + compute_expected(inputs[i], (crypto::pedersen_hash::lookup::NUM_PEDERSEN_TABLES / 2))) + .x; } EXPECT_EQ(affine_element(result).x, - affine_element(compute_expected(intermediate, 0) + compute_expected(fq(m), 15)).x); + affine_element(compute_expected(intermediate, 0) + + compute_expected(fq(m), (crypto::pedersen_hash::lookup::NUM_PEDERSEN_TABLES / 2))) + .x); } TEST(pedersen_lookup, merkle_damgard_compress_multiple_iv) @@ -164,14 +192,22 @@ TEST(pedersen_lookup, merkle_damgard_compress_multiple_iv) for (size_t i = 0; i < 2 * m; i++) { if ((i & 1) == 0) { const auto iv = (grumpkin::g1::affine_one * fr(ivs[i >> 1] + 1)).x; - intermediate = affine_element(compute_expected(intermediate, 0) + compute_expected(iv, 15)).x; + intermediate = + affine_element(compute_expected(intermediate, 0) + + compute_expected(iv, (crypto::pedersen_hash::lookup::NUM_PEDERSEN_TABLES / 2))) + .x; } else { - intermediate = affine_element(compute_expected(intermediate, 0) + compute_expected(inputs[i >> 1], 15)).x; + intermediate = affine_element(compute_expected(intermediate, 0) + + compute_expected(inputs[i >> 1], + (crypto::pedersen_hash::lookup::NUM_PEDERSEN_TABLES / 2))) + .x; } } EXPECT_EQ(affine_element(result).x, - affine_element(compute_expected(intermediate, 0) + compute_expected(fq(m), 15)).x); + affine_element(compute_expected(intermediate, 0) + + compute_expected(fq(m), (crypto::pedersen_hash::lookup::NUM_PEDERSEN_TABLES / 2))) + .x); } TEST(pedersen_lookup, merkle_damgard_tree_compress) @@ -193,16 +229,25 @@ TEST(pedersen_lookup, merkle_damgard_tree_compress) std::vector temp; for (size_t i = 0; i < m; i++) { const fq iv_term = (grumpkin::g1::affine_one * fr(ivs[i] + 1)).x; - temp.push_back(affine_element(compute_expected(iv_term, 0) + compute_expected(inputs[i], 15)).x); + temp.push_back( + affine_element(compute_expected(iv_term, 0) + + compute_expected(inputs[i], (crypto::pedersen_hash::lookup::NUM_PEDERSEN_TABLES / 2))) + .x); } const size_t logm = numeric::get_msb(m); for (size_t j = 1; j <= logm; j++) { const size_t nodes = (1UL << (logm - j)); for (size_t i = 0; i < nodes; i++) { - temp[i] = affine_element(compute_expected(temp[2 * i], 0) + compute_expected(temp[2 * i + 1], 15)).x; + temp[i] = affine_element( + compute_expected(temp[2 * i], 0) + + compute_expected(temp[2 * i + 1], (crypto::pedersen_hash::lookup::NUM_PEDERSEN_TABLES / 2))) + .x; } } - EXPECT_EQ(affine_element(result).x, affine_element(compute_expected(temp[0], 0) + compute_expected(fq(m), 15)).x); + EXPECT_EQ(affine_element(result).x, + affine_element(compute_expected(temp[0], 0) + + compute_expected(fq(m), (crypto::pedersen_hash::lookup::NUM_PEDERSEN_TABLES / 2))) + .x); } diff --git a/cpp/src/barretenberg/ecc/curves/bn254/fr.test.cpp b/cpp/src/barretenberg/ecc/curves/bn254/fr.test.cpp index 2f7a3a5f9f..683b491718 100644 --- a/cpp/src/barretenberg/ecc/curves/bn254/fr.test.cpp +++ b/cpp/src/barretenberg/ecc/curves/bn254/fr.test.cpp @@ -86,6 +86,35 @@ TEST(fr, sub) EXPECT_EQ((result == expected), true); } +TEST(fr, plus_equals) +{ + fr a{ 0x5def, 0x00, 0x00, 0x00 }; + fr a_copy = a; + a += 2; + fr expected = a_copy + 2; + EXPECT_EQ((a == expected), true); + + a += 3; + expected = a_copy + 5; + EXPECT_EQ((a == expected), true); +} + +TEST(fr, prefix_increment) +{ + fr a{ 0x5def, 0x00, 0x00, 0x00 }; + fr b = ++a; + EXPECT_EQ(b, a); +} + +TEST(fr, postfix_increment) +{ + fr a{ 0x5def, 0x00, 0x00, 0x00 }; + fr a_old = a; + fr b = a++; + EXPECT_EQ(b, a_old); + EXPECT_EQ(a, a_old + 1); +} + TEST(fr, to_montgomery_form) { fr result{ 0x01, 0x00, 0x00, 0x00 }; diff --git a/cpp/src/barretenberg/ecc/curves/bn254/g1.test.cpp b/cpp/src/barretenberg/ecc/curves/bn254/g1.test.cpp index f7fd7fb8df..e636e8f955 100644 --- a/cpp/src/barretenberg/ecc/curves/bn254/g1.test.cpp +++ b/cpp/src/barretenberg/ecc/curves/bn254/g1.test.cpp @@ -145,6 +145,15 @@ TEST(g1, add_exception_test_infinity) EXPECT_EQ(rhs == result, true); } +TEST(g1, test_infinity) +{ + g1::affine_element inf_affine = g1::affine_element::infinity(); + EXPECT_EQ(inf_affine.is_point_at_infinity(), true); + + g1::element inf_element = g1::element::infinity(); + EXPECT_EQ(inf_element.is_point_at_infinity(), true); +} + TEST(g1, add_exception_test_dbl) { g1::element lhs = g1::element::random_element(); @@ -160,6 +169,19 @@ TEST(g1, add_exception_test_dbl) EXPECT_EQ(result == expected, true); } +TEST(g1, add_affine_test) +{ + g1::element lhs = g1::element::random_element(); + g1::affine_element lhs_affine = g1::affine_element(lhs); + + g1::element rhs = g1::element::random_element(); + g1::affine_element rhs_affine = g1::affine_element(rhs); + + g1::element expected = lhs + rhs; + g1::affine_element result = lhs_affine + rhs_affine; + EXPECT_EQ(g1::element(result) == expected, true); +} + TEST(g1, add_dbl_consistency) { g1::element a = g1::element::random_element(); diff --git a/cpp/src/barretenberg/ecc/fields/field_impl.hpp b/cpp/src/barretenberg/ecc/fields/field_impl.hpp index c02eadb09f..057a7e4463 100644 --- a/cpp/src/barretenberg/ecc/fields/field_impl.hpp +++ b/cpp/src/barretenberg/ecc/fields/field_impl.hpp @@ -136,8 +136,9 @@ template constexpr field field::operator++() noexcept template constexpr field field::operator++(int) noexcept { - // TODO check if this is correct. - return *this += 1; + field value_before_incrementing = *this; + *this += 1; + return value_before_incrementing; } /** diff --git a/cpp/src/barretenberg/ecc/groups/element_impl.hpp b/cpp/src/barretenberg/ecc/groups/element_impl.hpp index baaaa36ec5..c3a7859530 100644 --- a/cpp/src/barretenberg/ecc/groups/element_impl.hpp +++ b/cpp/src/barretenberg/ecc/groups/element_impl.hpp @@ -489,9 +489,6 @@ template element element::in { element e; e.self_set_infinity(); - if (!e.is_point_at_infinity()) { - info("yup, it's infinity"); - }; return e; } diff --git a/cpp/src/barretenberg/stdlib/primitives/bool/bool.cpp b/cpp/src/barretenberg/stdlib/primitives/bool/bool.cpp index ba7c678fbf..43082f69a9 100644 --- a/cpp/src/barretenberg/stdlib/primitives/bool/bool.cpp +++ b/cpp/src/barretenberg/stdlib/primitives/bool/bool.cpp @@ -461,8 +461,38 @@ void bool_t::must_imply(const bool_t& other, std::string const& template void bool_t::must_imply(const std::vector>& conds) const { - bool_t acc = true; // will accumulate the conjunctions of each condition (i.e. `&&` of each) + // Extract the composer + const bool_t this_bool = *this; + ComposerContext* ctx = this_bool.get_context(); + bool composer_found = (ctx != nullptr); + for (size_t i = 0; i < conds.size(); i++) { + if (!composer_found) { + ctx = conds[i].first.get_context(); + composer_found = (ctx != nullptr); + } + } + + // If no composer is found, all of the bool_t's must be constants. + // In that case, we enforce a static assertion to check must_imply condition holds. + // If all of your inputs do this function are constants and they don't obey a condition, + // this function will panic at those static assertions. + if (!composer_found) { + bool is_const = this_bool.is_constant(); + bool result = !this_bool.get_value(); + bool acc = true; + for (size_t i = 0; i < conds.size(); i++) { + is_const &= conds[i].first.is_constant(); + acc &= conds[i].first.get_value(); + } + result |= acc; + ASSERT(is_const == true); + ASSERT(result == true); + } + + bool_t acc = witness_t(ctx, true); // will accumulate the conjunctions of each condition (i.e. `&&` of each) const bool this_val = this->get_value(); + bool error_found = false; + std::string error_msg; for (size_t i = 0; i < conds.size(); ++i) { const bool_t& cond = conds[i].first; @@ -470,16 +500,15 @@ void bool_t::must_imply(const std::vectorfailure(msg); + if (!(!this_val || cond_val) && !error_found) { + error_found = true; + error_msg = msg; } acc &= cond; } - (this->implies(acc)).assert_equal(true, "multi implication fail"); + (this->implies(acc)).assert_equal(true, format("multi implication fail: ", error_msg)); } // A "double-implication" (<=>), diff --git a/cpp/src/barretenberg/stdlib/primitives/bool/bool.test.cpp b/cpp/src/barretenberg/stdlib/primitives/bool/bool.test.cpp index 2d92aefd53..96db1b3cf2 100644 --- a/cpp/src/barretenberg/stdlib/primitives/bool/bool.test.cpp +++ b/cpp/src/barretenberg/stdlib/primitives/bool/bool.test.cpp @@ -1,13 +1,17 @@ #include "bool.hpp" #include "barretenberg/plonk/proof_system/constants.hpp" #include -// #include +#include "barretenberg/plonk/composer/standard_composer.hpp" #include "barretenberg/honk/composer/standard_honk_composer.hpp" namespace test_stdlib_bool { using namespace barretenberg; using namespace plonk; +namespace { +auto& engine = numeric::random::get_debug_engine(); +} + typedef stdlib::bool_t bool_t; typedef stdlib::witness_t witness_t; @@ -359,6 +363,142 @@ TEST(stdlib_bool, implies_both_ways) EXPECT_EQ(result, true); } +TEST(stdlib_bool, must_imply) +{ + honk::StandardHonkComposer composer = honk::StandardHonkComposer(); + for (size_t j = 0; j < 4; ++j) { + bool lhs_constant = (bool)(j % 2); + bool rhs_constant = (bool)(j > 1 ? true : false); + + for (size_t i = 4; i < 14; i += 2) { + // If a number is divisible by 2 and 3, it is divisible by 6 + bool two = (bool)(i % 2); + bool three = (bool)(i % 3); + bool six = (bool)(i % 6); + bool a_val = (two && three); + bool b_val = six; + bool_t a = lhs_constant ? bool_t(a_val) : (witness_t(&composer, a_val)); + bool_t b = rhs_constant ? bool_t(b_val) : (witness_t(&composer, b_val)); + a.must_imply(b); + } + } + 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); +} + +TEST(stdlib_bool, must_imply_multiple) +{ + honk::StandardHonkComposer composer = honk::StandardHonkComposer(); + + /** + * Define g(x) = 2x + 12 + * if x is divisible by both 4 and 6: + * => g(x) > 0 + * => g(x) is even + * => g(x) >= 12 + * => g(x) is a multiple of 6 + */ + auto g = [](size_t x) { return 2 * x + 12; }; + + for (size_t j = 0; j < 3; ++j) { // ignore when both lhs and rhs are constants + bool lhs_constant = (bool)(j % 2); + bool rhs_constant = (bool)(j > 1 ? true : false); + + for (size_t x = 10; x < 18; x += 2) { + std::vector> conditions; + bool four = (bool)(x % 4 == 0); + bool six = (bool)(x % 6 == 0); + + bool_t a = lhs_constant ? bool_t(four) : (witness_t(&composer, four)); + bool_t b = rhs_constant ? bool_t(six) : (witness_t(&composer, six)); + + auto g_x = g(x); + conditions.push_back(std::make_pair(g_x > 0, "g(x) > 0")); + conditions.push_back(std::make_pair(g_x % 2 == 0, "g(x) is even")); + conditions.push_back(std::make_pair(g_x >= 12, "g(x) >= 12")); + conditions.push_back(std::make_pair(g_x % 6 == 0, "g(x) is a multiple of 6")); + + (a && b).must_imply(conditions); + + if (composer.failed()) { + EXPECT_EQ(composer.err(), "multi implication fail: g(x) is a multiple of 6"); + } else { + 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); + } + } + } +} + +TEST(stdlib_bool, must_imply_multiple_fails) +{ + honk::StandardHonkComposer composer = honk::StandardHonkComposer(); + + /** + * Given x = 15: + * (x > 10) + * => (x > 8) + * => (x > 5) + * ≠> (x > 18) + */ + for (size_t j = 0; j < 2; ++j) { // ignore when both lhs and rhs are constants + bool is_constant = (bool)(j % 2); + + size_t x = 15; + bool main = (bool)(x > 10); + bool_t main_ct = is_constant ? bool_t(main) : (witness_t(&composer, main)); + + std::vector> conditions; + conditions.push_back(std::make_pair(witness_t(&composer, x > 8), "x > 8")); + conditions.push_back(std::make_pair(witness_t(&composer, x > 5), "x > 5")); + conditions.push_back(std::make_pair(witness_t(&composer, x > 18), "x > 18")); + + main_ct.must_imply(conditions); + + EXPECT_EQ(composer.failed(), true); + EXPECT_EQ(composer.err(), "multi implication fail: x > 18"); + } +} + +TEST(stdlib_bool, conditional_assign) +{ + honk::StandardHonkComposer composer = honk::StandardHonkComposer(); + for (size_t j = 0; j < 4; ++j) { + bool lhs_constant = (bool)(j % 2); + bool rhs_constant = (bool)(j > 1 ? true : false); + + const uint256_t x = (uint256_t(1) << 128) - 1; + const uint256_t val = engine.get_random_uint256(); + + bool condition = (val % 2 == 0); + bool right = x < val; + bool left = x > val; + bool_t l_ct = lhs_constant ? bool_t(left) : (witness_t(&composer, left)); + bool_t r_ct = rhs_constant ? bool_t(right) : (witness_t(&composer, right)); + bool_t cond = (witness_t(&composer, condition)); + + auto result = bool_t::conditional_assign(cond, l_ct, r_ct); + + EXPECT_EQ(result.get_value(), condition ? left : right); + } + auto prover = composer.create_prover(); + auto verifier = composer.create_verifier(); + + plonk::proof proof = prover.construct_proof(); + info("composer gates = ", composer.get_num_gates()); + bool result = verifier.verify_proof(proof); + EXPECT_EQ(result, true); +} + TEST(stdlib_bool, test_simple_proof) { honk::StandardHonkComposer composer = honk::StandardHonkComposer(); diff --git a/cpp/src/barretenberg/stdlib/primitives/field/field.test.cpp b/cpp/src/barretenberg/stdlib/primitives/field/field.test.cpp index 6b6da2b4df..ebd46d5936 100644 --- a/cpp/src/barretenberg/stdlib/primitives/field/field.test.cpp +++ b/cpp/src/barretenberg/stdlib/primitives/field/field.test.cpp @@ -947,6 +947,30 @@ template class stdlib_field : public testing::Test { EXPECT_EQ(composer.failed(), true); EXPECT_EQ(composer.err(), "field_t::pow exponent accumulator incorrect"); }; + + static void test_copy_as_new_witness() + { + Composer composer = Composer(); + + barretenberg::fr value(engine.get_random_uint256()); + field_ct value_ct = witness_ct(&composer, value); + + field_ct first_copy = witness_ct(&composer, value_ct.get_value()); + field_ct second_copy = field_ct::copy_as_new_witness(composer, value_ct); + + EXPECT_EQ(value_ct.get_value(), value); + EXPECT_EQ(first_copy.get_value(), value); + EXPECT_EQ(second_copy.get_value(), value); + EXPECT_EQ(value_ct.get_witness_index() + 1, first_copy.get_witness_index()); + EXPECT_EQ(value_ct.get_witness_index() + 2, second_copy.get_witness_index()); + + auto prover = composer.create_prover(); + auto verifier = composer.create_verifier(); + auto proof = prover.construct_proof(); + info("composer gates = ", composer.get_num_gates()); + bool proof_result = verifier.verify_proof(proof); + EXPECT_EQ(proof_result, true); + } }; typedef testing::Types @@ -1070,4 +1094,8 @@ TYPED_TEST(stdlib_field, test_pow_exponent_out_of_range) { TestFixture::test_pow_exponent_out_of_range(); } +TYPED_TEST(stdlib_field, test_copy_as_new_witness) +{ + TestFixture::test_copy_as_new_witness(); +} } // namespace test_stdlib_field