From 000c71a718db292b4cc8e8187ffb83bf197c4e85 Mon Sep 17 00:00:00 2001 From: Guido Vranken Date: Thu, 24 Nov 2022 11:16:17 +0100 Subject: [PATCH 01/23] Set context of return value in bigfield division (#1785) Fixes https://github.com/AztecProtocol/aztec2-internal/issues/1784 --- .../stdlib/primitives/bigfield/bigfield.test.cpp | 15 +++++++++++++++ .../stdlib/primitives/bigfield/bigfield_impl.hpp | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/aztec/stdlib/primitives/bigfield/bigfield.test.cpp b/src/aztec/stdlib/primitives/bigfield/bigfield.test.cpp index 34771757914..a9b73eaaa54 100644 --- a/src/aztec/stdlib/primitives/bigfield/bigfield.test.cpp +++ b/src/aztec/stdlib/primitives/bigfield/bigfield.test.cpp @@ -791,6 +791,15 @@ template class stdlib_bigfield : public testing::Test { fq_ct selected = a_ct.conditional_select(b_ct, typename bn254::bool_ct(&composer, true)); EXPECT_EQ(barretenberg::fq((selected.get_value() % uint512_t(barretenberg::fq::modulus)).lo), b); } + + static void test_division_context() + { + auto composer = Composer(); + barretenberg::fq a(1); + fq_ct a_ct(&composer, a); + fq_ct ret = fq_ct::div_check_denominator_nonzero({}, a_ct); + EXPECT_NE(ret.get_context(), nullptr); + } }; // Define types for which the above tests will be constructed. @@ -870,6 +879,12 @@ TYPED_TEST(stdlib_bigfield, conditional_select_regression) { TestFixture::test_conditional_select_regression(); } + +TYPED_TEST(stdlib_bigfield, division_context) +{ + TestFixture::test_division_context(); +} + // // This test was disabled before the refactor to use TYPED_TEST's/ // TEST(stdlib_bigfield, DISABLED_test_div_against_constants) // { diff --git a/src/aztec/stdlib/primitives/bigfield/bigfield_impl.hpp b/src/aztec/stdlib/primitives/bigfield/bigfield_impl.hpp index 7e0f6dfc347..6e169304b26 100644 --- a/src/aztec/stdlib/primitives/bigfield/bigfield_impl.hpp +++ b/src/aztec/stdlib/primitives/bigfield/bigfield_impl.hpp @@ -512,7 +512,7 @@ bigfield bigfield::internal_div(const std::vector& numerat bool check_for_zero) { if (numerators.size() == 0) { - return bigfield(nullptr, uint256_t(0)); + return bigfield(denominator.get_context(), uint256_t(0)); } denominator.reduction_check(); From 37adf5c9c06c3cafb034a79717285fbd2de0149a Mon Sep 17 00:00:00 2001 From: Lasse Herskind <16536249+LHerskind@users.noreply.github.com> Date: Mon, 28 Nov 2022 10:57:57 +0100 Subject: [PATCH 02/23] Verification Key changes (#1695) * Account bug fix (#1490) * fix account bug - remove y coordinate for public key nullifier * fix account bug in all relevant places in barretenberg * fix note_algorithms test * account circuit doc update for nullifier Y coord removal (account bug) * Added account nullifier content in notes_and_nullifiers.md. Fixed up some formatting in account_circuit.md and possibly fixed a typo * fixed up formatting in account circuit and notes/nullifiers docs * updated Verification Key since circuits were updated * fix: Build verification keys * fix: update circuit constants * reverting the root rollup constants changes * fix: update vk_hash in UpgradeV2 + fmt * Bigfield formula fix for single division (#1610) * Added division formula bug test * Fixed the single div bug by adding an unreduced zero to the formula * Trigger rebuild * Updated root rollup size * Update rollup names and add tests to more jobs * Updated some hashes and 1 test * Updated hashes and gate counts * Updated rollup gates * Revert adding extra information to log * Remove the cosmetic change * Updated hashes * fix: generate vks * fix: update `PROD_VK_HASH` in UpgradeV2 * check signing key on curve, and rename grumpkin.one (#1781) * check signing key on curve, and rename grumpkin.one * Rm commented-out line * update gate count & vkhash of js circuit * update vkhashes of all rollup circuits * Add `on_curve` check in stdlib point. * Update circuit keys. * Change root verifier circuit vk hash. * fix: generate vk contracts + upgrade prod_key_hash Co-authored-by: Suyash Bagad Co-authored-by: LHerskind Co-authored-by: David Banks <47112877+dbanks12@users.noreply.github.com> Co-authored-by: arijitdutta67 Co-authored-by: Innokentii Sennovskii Co-authored-by: Michael Connor Co-authored-by: Suyash Bagad --- src/aztec/rollup/constants.hpp | 22 +++++++++---------- src/aztec/rollup/proofs/account/account.cpp | 2 +- .../rollup/proofs/account/account.test.cpp | 2 +- .../rollup/proofs/account/account_tx.cpp | 2 +- .../proofs/join_split/join_split_circuit.cpp | 5 ++--- .../compute_account_public_key_nullifier.hpp | 4 ++-- .../rollup/rollup_circuit_full.test.cpp | 12 +++++----- .../stdlib/encryption/schnorr/schnorr.cpp | 6 +++++ .../stdlib/primitives/bigfield/bigfield.hpp | 22 +++++++++++++++++++ .../primitives/bigfield/bigfield.test.cpp | 21 ++++++++++++++++++ .../primitives/bigfield/bigfield_impl.hpp | 7 +++--- src/aztec/stdlib/primitives/point/point.hpp | 14 ++++++++---- 12 files changed, 87 insertions(+), 32 deletions(-) diff --git a/src/aztec/rollup/constants.hpp b/src/aztec/rollup/constants.hpp index d08013b15b2..9275e9c82a4 100644 --- a/src/aztec/rollup/constants.hpp +++ b/src/aztec/rollup/constants.hpp @@ -34,19 +34,19 @@ is_circuit_change_expected to zero and change the modified circuit gate counts a constexpr bool is_circuit_change_expected = 0; /* The below constants are only used for regression testing; to identify accidental changes to circuit constraints. They need to be changed when there is a circuit change. */ -constexpr uint32_t JOIN_SPLIT = 63984; -constexpr uint32_t ACCOUNT = 24123; +constexpr uint32_t ACCOUNT = 23958; +constexpr uint32_t JOIN_SPLIT = 64000; constexpr uint32_t CLAIM = 22684; -constexpr uint32_t ROLLUP = 1171925; -constexpr uint32_t ROOT_ROLLUP = 5477579; -constexpr uint32_t ROOT_VERIFIER = 7433260; +constexpr uint32_t ROLLUP = 1173221; +constexpr uint32_t ROOT_ROLLUP = 5481327; +constexpr uint32_t ROOT_VERIFIER = 7435892; }; // namespace circuit_gate_count namespace circuit_gate_next_power_of_two { /* The below constants are used in tests to detect undesirable circuit changes. They should not be changed unless we want to exceed the next power of two limit. */ -constexpr uint32_t JOIN_SPLIT = 65536; constexpr uint32_t ACCOUNT = 32768; +constexpr uint32_t JOIN_SPLIT = 65536; constexpr uint32_t CLAIM = 32768; constexpr uint32_t ROLLUP = 2097152; constexpr uint32_t ROOT_ROLLUP = 8388608; @@ -57,13 +57,13 @@ namespace circuit_vk_hash { /* These below constants are only used for regression testing; to identify accidental changes to circuit constraints. They need to be changed when there is a circuit change. Note that they are written in the reverse order to comply with the from_buffer<>() method. */ -constexpr auto ACCOUNT = uint256_t(0x5d97584af28e524a, 0xd2007df20f73d2bf, 0xe134d5b853eff05a, 0xdd2c3d1ff8bce519); -constexpr auto JOIN_SPLIT = uint256_t(0x7d54acfcca0eaf34, 0xa69c7b4e1c2fb5c5, 0x77161b356ca1c7a2, 0xb88194f268d2caa9); +constexpr auto ACCOUNT = uint256_t(0x78ebf096ab73e440, 0xaa1dc7c26a125f6e, 0x488a97e465b96964, 0xf9d3e501b89bf466); +constexpr auto JOIN_SPLIT = uint256_t(0x5e67a4a4503ebf25, 0xb3c070c061e76d1a, 0xb18c6c6a5bcad5fb, 0xe0d5f46cafb33ecf); constexpr auto CLAIM = uint256_t(0x878301ebba40ab60, 0x931466762c62d661, 0x40aad71ec3496905, 0x9f47aaa109759d0a); -constexpr auto ROLLUP = uint256_t(0x3c9e491095547852, 0xbf65ec889ec96a1a, 0xb16e824aa0bb319f, 0x28d7b587edf1eb4d); -constexpr auto ROOT_ROLLUP = uint256_t(0xa5e06b55f0e30cbe, 0x5fbf39af52fe67c8, 0xd8a0ecd1bb3a6f40, 0xdf67f7fcbb55dc1f); +constexpr auto ROLLUP = uint256_t(0x160731cc44173fdc, 0x6a6d55e46bf198bd, 0x9ce1d4608ae26fb0, 0x865ced5c16cb6152); +constexpr auto ROOT_ROLLUP = uint256_t(0xd77e82eae9e6efc7, 0x2b5ddf767012a4cf, 0x8b5982bb3d64616f, 0x20b515f5a9c78048); constexpr auto ROOT_VERIFIER = - uint256_t(0x341a876aae2df472, 0x87e0704f1ae50773, 0x5d6c740f61a0dbdd, 0x1f1a94b50cdcf5ae); + uint256_t(0x8e8313d6015ca626, 0x62ccf70b81c4e099, 0x33bee0072a20f36a, 0x44bd24daa009cd59); }; // namespace circuit_vk_hash namespace ProofIds { diff --git a/src/aztec/rollup/proofs/account/account.cpp b/src/aztec/rollup/proofs/account/account.cpp index b8c7edbaa04..03173f5c273 100644 --- a/src/aztec/rollup/proofs/account/account.cpp +++ b/src/aztec/rollup/proofs/account/account.cpp @@ -33,7 +33,7 @@ field_ct compute_account_alias_hash_nullifier(suint_ct const& account_alias_hash field_ct compute_account_public_key_nullifier(point_ct const& account_public_key) { - return pedersen::compress(std::vector{ account_public_key.x, account_public_key.y }, + return pedersen::compress(std::vector{ account_public_key.x }, notes::GeneratorIndex::ACCOUNT_PUBLIC_KEY_NULLIFIER); } void account_circuit(Composer& composer, account_tx const& tx) diff --git a/src/aztec/rollup/proofs/account/account.test.cpp b/src/aztec/rollup/proofs/account/account.test.cpp index 5896aac8128..12fca78d8fc 100644 --- a/src/aztec/rollup/proofs/account/account.test.cpp +++ b/src/aztec/rollup/proofs/account/account.test.cpp @@ -68,7 +68,7 @@ class account_tests : public ::testing::Test { uint256_t compute_account_public_key_nullifier(grumpkin::g1::affine_element const& account_public_key) { - return crypto::pedersen::compress_native({ account_public_key.x, account_public_key.y }, + return crypto::pedersen::compress_native({ account_public_key.x }, notes::GeneratorIndex::ACCOUNT_PUBLIC_KEY_NULLIFIER); } diff --git a/src/aztec/rollup/proofs/account/account_tx.cpp b/src/aztec/rollup/proofs/account/account_tx.cpp index 1d419305cdd..99a44ed4483 100644 --- a/src/aztec/rollup/proofs/account/account_tx.cpp +++ b/src/aztec/rollup/proofs/account/account_tx.cpp @@ -21,7 +21,7 @@ fr account_tx::compute_account_alias_hash_nullifier() const fr account_tx::compute_account_public_key_nullifier() const { if (create || migrate) { - return compress_native({ new_account_public_key.x, new_account_public_key.y }, + return compress_native({ new_account_public_key.x }, rollup::proofs::notes::GeneratorIndex::ACCOUNT_PUBLIC_KEY_NULLIFIER); } return 0; diff --git a/src/aztec/rollup/proofs/join_split/join_split_circuit.cpp b/src/aztec/rollup/proofs/join_split/join_split_circuit.cpp index 91a7d803a21..468a43e6985 100644 --- a/src/aztec/rollup/proofs/join_split/join_split_circuit.cpp +++ b/src/aztec/rollup/proofs/join_split/join_split_circuit.cpp @@ -287,8 +287,7 @@ void join_split_circuit(Composer& composer, join_split_tx const& tx) // Construction of partial_claim_note_witness_data includes construction of bridge_call_data, which contains // many constraints on the bridge_call_data's format and the bit_config's format: .partial_claim_note = claim::partial_claim_note_witness_data(composer, tx.partial_claim_note), - .signing_pub_key = { .x = witness_ct(&composer, tx.signing_pub_key.x), - .y = witness_ct(&composer, tx.signing_pub_key.y) }, + .signing_pub_key = stdlib::create_point_witness(composer, tx.signing_pub_key), .signature = stdlib::schnorr::convert_signature(&composer, tx.signature), .merkle_root = witness_ct(&composer, tx.old_data_root), .input_path1 = merkle_tree::create_witness_hash_path(composer, tx.input_path[0]), @@ -326,7 +325,7 @@ void join_split_circuit(Composer& composer, join_split_tx const& tx) defi_root.set_public(); inputs.backward_link.set_public(); inputs.allow_chain.set_public(); -} +} // namespace join_split } // namespace join_split } // namespace proofs diff --git a/src/aztec/rollup/proofs/notes/native/account/compute_account_public_key_nullifier.hpp b/src/aztec/rollup/proofs/notes/native/account/compute_account_public_key_nullifier.hpp index a9d9ce5a2e6..03e93b90fff 100644 --- a/src/aztec/rollup/proofs/notes/native/account/compute_account_public_key_nullifier.hpp +++ b/src/aztec/rollup/proofs/notes/native/account/compute_account_public_key_nullifier.hpp @@ -13,7 +13,7 @@ using namespace barretenberg; inline fr compute_account_public_key_nullifier(grumpkin::g1::affine_element const& public_key) { - return crypto::pedersen::compress_native(std::vector{ public_key.x, public_key.y }, + return crypto::pedersen::compress_native(std::vector{ public_key.x }, notes::GeneratorIndex::ACCOUNT_PUBLIC_KEY_NULLIFIER); } @@ -21,4 +21,4 @@ inline fr compute_account_public_key_nullifier(grumpkin::g1::affine_element cons } // namespace native } // namespace notes } // namespace proofs -} // namespace rollup \ No newline at end of file +} // namespace rollup diff --git a/src/aztec/rollup/proofs/rollup/rollup_circuit_full.test.cpp b/src/aztec/rollup/proofs/rollup/rollup_circuit_full.test.cpp index c4f27a2f260..2f1568af13d 100644 --- a/src/aztec/rollup/proofs/rollup/rollup_circuit_full.test.cpp +++ b/src/aztec/rollup/proofs/rollup/rollup_circuit_full.test.cpp @@ -90,18 +90,18 @@ HEAVY_TEST_F(rollup_full_tests, test_1_proof_in_1_rollup_full_proof_and_detect_c // rollup/constants.hpp and see if atleast the next power of two limit is not exceeded. Please change the constant // values accordingly and set is_circuit_change_expected to 0 in rollup/constants.hpp before merging. if (!(circuit_gate_count::is_circuit_change_expected)) { - EXPECT_TRUE(number_of_gates_rollup == circuit_gate_count::ROLLUP) + EXPECT_EQ(number_of_gates_rollup, circuit_gate_count::ROLLUP) << "The gate count for the rollup circuit is changed."; - EXPECT_TRUE(from_buffer(vk_hash_rollup) == circuit_vk_hash::ROLLUP) + EXPECT_EQ(from_buffer(vk_hash_rollup), circuit_vk_hash::ROLLUP) << "The verification key hash for the rollup circuit is changed."; // For the next power of two limit, we need to consider that we reserve four gates for adding // randomness/zero-knowledge - EXPECT_TRUE(number_of_gates_rollup <= - circuit_gate_next_power_of_two::ROLLUP - waffle::ComposerBase::NUM_RESERVED_GATES) + EXPECT_LE(number_of_gates_rollup, + circuit_gate_next_power_of_two::ROLLUP - waffle::ComposerBase::NUM_RESERVED_GATES) << "You have exceeded the next power of two limit for the rollup circuit."; } else { - EXPECT_TRUE(number_of_gates_rollup <= - circuit_gate_next_power_of_two::ROLLUP - waffle::ComposerBase::NUM_RESERVED_GATES) + EXPECT_LE(number_of_gates_rollup, + circuit_gate_next_power_of_two::ROLLUP - waffle::ComposerBase::NUM_RESERVED_GATES) << "You have exceeded the next power of two limit for the rollup circuit."; } } diff --git a/src/aztec/stdlib/encryption/schnorr/schnorr.cpp b/src/aztec/stdlib/encryption/schnorr/schnorr.cpp index 71742b85afc..de0f4992c1c 100644 --- a/src/aztec/stdlib/encryption/schnorr/schnorr.cpp +++ b/src/aztec/stdlib/encryption/schnorr/schnorr.cpp @@ -146,6 +146,9 @@ point variable_base_mul(const point& pub_key, const field_t& low_bits, template point variable_base_mul(const point& pub_key, const point& current_accumulator, const wnaf_record& wnaf) { + // Check if the pub_key is a points on the curve. + pub_key.on_curve(); + // The account circuit constrains `pub_key` to lie on Grumpkin. Presently, the only values that are passed in the // second argument as `current_accumulator` are `pub_key` and a point which is the output of the present function. // We therefore assume that `current_accumulator` lies on Grumpkin as well. @@ -168,6 +171,9 @@ point variable_base_mul(const point& pub_key, const point& current_accu if (init) { field_t zero_test = ((pub_key.x - collision_offset.x) * (pub_key.y - collision_offset.y)); zero_test.assert_is_not_zero("pub_key and collision_offset have a coordinate in common."); + } else { + // Check if the current_accumulator is a point on the curve only if init is false. + current_accumulator.on_curve(); } point accumulator{ collision_offset.x, collision_offset.y }; diff --git a/src/aztec/stdlib/primitives/bigfield/bigfield.hpp b/src/aztec/stdlib/primitives/bigfield/bigfield.hpp index 4d16a684d3c..a3974cb47c1 100644 --- a/src/aztec/stdlib/primitives/bigfield/bigfield.hpp +++ b/src/aztec/stdlib/primitives/bigfield/bigfield.hpp @@ -254,6 +254,28 @@ template class bigfield { return result; } + /** + * @brief Create an unreduced 0 ~ p*k, where p*k is the minimal multiple of modulus that should be reduced + * + * @details We need it for division. If we always add this element during division, then we never run into the + * formula-breaking situation + */ + static constexpr bigfield unreduced_zero() + { + uint512_t multiple_of_modulus = ((get_maximum_unreduced_value() / modulus_u512) + 1) * modulus_u512; + auto msb = multiple_of_modulus.get_msb(); + + bigfield result(nullptr, uint256_t(0)); + result.binary_basis_limbs[0] = Limb(barretenberg::fr(multiple_of_modulus.slice(0, NUM_LIMB_BITS).lo)); + result.binary_basis_limbs[1] = + Limb(barretenberg::fr(multiple_of_modulus.slice(NUM_LIMB_BITS, 2 * NUM_LIMB_BITS).lo)); + result.binary_basis_limbs[2] = + Limb(barretenberg::fr(multiple_of_modulus.slice(2 * NUM_LIMB_BITS, 3 * NUM_LIMB_BITS).lo)); + result.binary_basis_limbs[3] = Limb(barretenberg::fr(multiple_of_modulus.slice(3 * NUM_LIMB_BITS, msb + 1).lo)); + result.prime_basis_limb = field_t((multiple_of_modulus % uint512_t(field_t::modulus)).lo); + return result; + } + /** * Create a witness form a constant. This way the value of the witness is fixed and public. **/ diff --git a/src/aztec/stdlib/primitives/bigfield/bigfield.test.cpp b/src/aztec/stdlib/primitives/bigfield/bigfield.test.cpp index a9b73eaaa54..251fc45de0b 100644 --- a/src/aztec/stdlib/primitives/bigfield/bigfield.test.cpp +++ b/src/aztec/stdlib/primitives/bigfield/bigfield.test.cpp @@ -47,6 +47,23 @@ template class stdlib_bigfield : public testing::Test { typedef typename bn254::witness_ct witness_ct; public: + // The bug happens when we are applying the CRT formula to a*b < r, which can happen when using the division + // operator + static void test_division_formula_bug() + { + auto composer = Composer(); + uint256_t value(2); + fq_ct tval = fq_ct::create_from_u512_as_witness(&composer, value); + fq_ct tval1 = tval - tval; + fq_ct tval2 = tval1 / tval; + (void)tval2; + auto prover = composer.create_prover(); + auto verifier = composer.create_verifier(); + waffle::plonk_proof proof = prover.construct_proof(); + bool proof_result = verifier.verify_proof(proof); + EXPECT_EQ(proof_result, true); + } + static void test_bad_mul() { @@ -810,6 +827,10 @@ typedef testing::Types bigfield::internal_div(const std::vector& numerat uint1024_t inverse_1024(inverse_value); inverse_value = ((left * inverse_1024) % modulus).lo; - const uint1024_t quotient_1024 = (uint1024_t(inverse_value) * right - left) / modulus; + const uint1024_t quotient_1024 = + (uint1024_t(inverse_value) * right + unreduced_zero().get_value() - left) / modulus; const uint512_t quotient_value = quotient_1024.lo; bigfield inverse; @@ -555,7 +556,7 @@ bigfield bigfield::internal_div(const std::vector& numerat auto [reduction_required, num_quotient_bits] = get_quotient_reduction_info({ static_cast(DEFAULT_MAXIMUM_REMAINDER) }, { denominator.get_maximum_value() }, - {}, + { unreduced_zero() }, numerator_max); if (reduction_required) { @@ -575,7 +576,7 @@ bigfield bigfield::internal_div(const std::vector& numerat witness_t(ctx, fr(inverse_value.slice(NUM_LIMB_BITS * 2, NUM_LIMB_BITS * 3 + NUM_LAST_LIMB_BITS).lo))); } - unsafe_evaluate_multiply_add(denominator, inverse, {}, quotient, numerators); + unsafe_evaluate_multiply_add(denominator, inverse, { unreduced_zero() }, quotient, numerators); return inverse; } diff --git a/src/aztec/stdlib/primitives/point/point.hpp b/src/aztec/stdlib/primitives/point/point.hpp index d5d5ab41052..27aa9f88eff 100644 --- a/src/aztec/stdlib/primitives/point/point.hpp +++ b/src/aztec/stdlib/primitives/point/point.hpp @@ -24,6 +24,14 @@ template struct point { this->y.assert_equal(rhs.y, msg); } + void on_curve(std::string const& msg = "point::on_curve: point not on curve") const + { + auto on_curve = x * x; + on_curve = on_curve * x + grumpkin::g1::curve_b; // x^3 - 17 + on_curve = y.madd(y, -on_curve); // on_curve = y^2 - (x^3 - 17) == 0 + on_curve.assert_is_zero(msg); + } + void assert_not_equal(const point& rhs, std::string const& msg = "point:assert_not_equal") const { const auto lhs_eq = this->x == rhs.x; @@ -46,13 +54,11 @@ point create_point_witness(Composer& composer, E const& p, const bool // validate point is on the grumpkin curve field_t x(witness_t(&composer, p.x)); field_t y(witness_t(&composer, p.y)); + point result = { x, y }; // we need to disable this for when we are conditionally creating a point (e.g. account output note spending keys) if (validate_on_curve) { - auto on_curve = x * x; - on_curve = on_curve * x + grumpkin::g1::curve_b; // x^3 - 17 - on_curve = y.madd(y, -on_curve); // on_curve = y^2 - (x^3 - 17) == 0 - on_curve.assert_is_zero("create_point_witness: point not on curve"); + result.on_curve("create_point_witness: point not on curve"); } return { x, y }; } From d8d292148a5bfe9b8a24411ab0f17473bd5301a8 Mon Sep 17 00:00:00 2001 From: Suyash Bagad Date: Mon, 28 Nov 2022 22:56:36 +0530 Subject: [PATCH 03/23] Add a `noop` tx for account circuit. (#1808) --- .../proofs/account/compute_circuit_data.hpp | 37 ++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/src/aztec/rollup/proofs/account/compute_circuit_data.hpp b/src/aztec/rollup/proofs/account/compute_circuit_data.hpp index 8863576cf1e..5ac1c263b94 100644 --- a/src/aztec/rollup/proofs/account/compute_circuit_data.hpp +++ b/src/aztec/rollup/proofs/account/compute_circuit_data.hpp @@ -8,6 +8,41 @@ namespace rollup { namespace proofs { namespace account { +using namespace plonk::stdlib::merkle_tree; + +/** + * @brief Create an account noop transaction that sets the members in account_tx to be random/zero values. + * Note that the noop account tx satisfies the circuit logic, and hence can be used to create "dummy" account proofs + * that pass verification. + * + * @warning This must not be used in any production code! + */ +inline account_tx noop_tx() +{ + grumpkin::fr priv_key = grumpkin::fr::random_element(); + grumpkin::g1::affine_element pub_key = grumpkin::g1::one * priv_key; + + grumpkin::fr new_priv_key = grumpkin::fr::random_element(); + grumpkin::g1::affine_element new_pub_key = grumpkin::g1::one * new_priv_key; + + auto gibberish_path = fr_hash_path(DATA_TREE_DEPTH, std::make_pair(fr::random_element(), fr::random_element())); + + account_tx tx = {}; + tx.merkle_root = fr::random_element(); + tx.account_public_key = pub_key; + tx.new_account_public_key = pub_key; + tx.new_signing_pub_key_1 = new_pub_key; + tx.new_signing_pub_key_2 = new_pub_key; + tx.alias_hash = (uint256_t(fr::random_element()) & 0xffffffff); + tx.create = true; + tx.migrate = false; + tx.account_note_index = 0; + tx.signing_pub_key = pub_key; + tx.account_note_path = gibberish_path; + tx.sign({ priv_key, pub_key }); + return tx; +} + using circuit_data = proofs::circuit_data; inline circuit_data get_circuit_data(std::shared_ptr const& srs, bool mock = false) @@ -15,7 +50,7 @@ inline circuit_data get_circuit_data(std::shared_ptr Date: Thu, 1 Dec 2022 17:02:37 +0100 Subject: [PATCH 04/23] Fully initialize members in client_proofs_join_split_tx test (#1653) Fixes https://github.com/AztecProtocol/aztec2-internal/issues/1652 --- src/aztec/rollup/proofs/join_split/join_split_tx.test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/aztec/rollup/proofs/join_split/join_split_tx.test.cpp b/src/aztec/rollup/proofs/join_split/join_split_tx.test.cpp index 7bc5e124306..019ea292690 100644 --- a/src/aztec/rollup/proofs/join_split/join_split_tx.test.cpp +++ b/src/aztec/rollup/proofs/join_split/join_split_tx.test.cpp @@ -21,7 +21,7 @@ auto& engine = numeric::random::get_debug_engine(); TEST(client_proofs_join_split_tx, test_serialization) { - join_split_tx tx; + join_split_tx tx = {}; tx.proof_id = 1; tx.account_note_index = 0; tx.signing_pub_key = grumpkin::g1::one * grumpkin::fr::random_element(&engine); From 3e5188062beb010a3164ee5bf66fdf51b17ac43f Mon Sep 17 00:00:00 2001 From: adr1anh Date: Thu, 1 Dec 2022 13:43:43 +0000 Subject: [PATCH 05/23] CVF2 --- src/aztec/crypto/hmac/hmac.hpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/aztec/crypto/hmac/hmac.hpp b/src/aztec/crypto/hmac/hmac.hpp index 30b3a679415..295fd99c172 100644 --- a/src/aztec/crypto/hmac/hmac.hpp +++ b/src/aztec/crypto/hmac/hmac.hpp @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -21,6 +22,7 @@ template std::array hmac(const MessageContainer& message, const KeyContainer& key) { constexpr size_t B = Hash::BLOCK_SIZE; + static_assert(Hash::OUTPUT_SIZE <= B); std::array ipad; std::array opad; for (size_t i = 0; i < B; ++i) { From 302c9b33c2c1f04e27ec83a90865b48c60f89a71 Mon Sep 17 00:00:00 2001 From: adr1anh Date: Thu, 1 Dec 2022 13:49:38 +0000 Subject: [PATCH 06/23] CVF 1 --- src/aztec/crypto/hmac/hmac.hpp | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/src/aztec/crypto/hmac/hmac.hpp b/src/aztec/crypto/hmac/hmac.hpp index 295fd99c172..27f13275dfc 100644 --- a/src/aztec/crypto/hmac/hmac.hpp +++ b/src/aztec/crypto/hmac/hmac.hpp @@ -22,29 +22,25 @@ template std::array hmac(const MessageContainer& message, const KeyContainer& key) { constexpr size_t B = Hash::BLOCK_SIZE; + // ensures truncated_key fits into k_prime static_assert(Hash::OUTPUT_SIZE <= B); + constexpr uint8_t IPAD_CONST = 0x36; + constexpr uint8_t OPAD_CONST = 0x5c; std::array ipad; std::array opad; - for (size_t i = 0; i < B; ++i) { - opad[i] = 0x5c; - ipad[i] = 0x36; - } + ipad.fill(IPAD_CONST); + opad.fill(OPAD_CONST); - std::array k_prime; + // initialize k_prime to 0x00,...,0x00 + // copy key or truncated key to start. + std::array k_prime{}; if (key.size() > B) { const auto truncated_key = Hash::hash(key); std::copy(truncated_key.begin(), truncated_key.end(), k_prime.begin()); - for (size_t i = Hash::OUTPUT_SIZE; i < B; ++i) { - k_prime[i] = 0x00; - } } else { std::copy(key.begin(), key.end(), k_prime.begin()); - for (size_t i = key.size(); i < B; ++i) { - k_prime[i] = 0x00; - } } - // std::cout << uint8_to_hex_string(&k_prime[0], B) << std::endl; std::array h1; for (size_t i = 0; i < B; ++i) { h1[i] = k_prime[i] ^ opad[i]; From d9d9e289c76b6105e13b1df820f031b062291a3c Mon Sep 17 00:00:00 2001 From: adr1anh Date: Thu, 1 Dec 2022 14:17:00 +0000 Subject: [PATCH 07/23] CVF 5,6,7,8,10,11,12,13 --- src/aztec/crypto/hmac/hmac.hpp | 46 ++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/src/aztec/crypto/hmac/hmac.hpp b/src/aztec/crypto/hmac/hmac.hpp index 27f13275dfc..337974176c5 100644 --- a/src/aztec/crypto/hmac/hmac.hpp +++ b/src/aztec/crypto/hmac/hmac.hpp @@ -1,5 +1,6 @@ #pragma once +#include "common/serialize.hpp" #include #include #include @@ -71,36 +72,49 @@ std::array hmac(const MessageContainer& message, con /** * @brief Takes a size-HASH_OUTPUT buffer from HMAC and converts into a field element * - * @details We assume HASH_OUTPUT = 32, which is insufficient entropy. We hash input with `0` and `1` to produce 64 - * bytes of input data. This is then converted into a uin512_t, which is taken modulo Fr::modulus to produce our field - * element. + * @details We assume HASH_OUTPUT = 32. Reducing HMAC(key, message) modulo r would result in an unacceptable bias. + * We hash input with `0` and `1` to produce 64 bytes of input data. This is then converted into a uin512_t, + * which is taken modulo Fr::modulus to produce our field element, where the statistical bias is negligble in + * the security parameter. * * @tparam Hash the hash function we're using * @tparam Fr field type - * @param input the input buffer - * @return Fr output field element + * @tparam MessageContainer a byte container (std::vector, std::array, std::string) + * @tparam KeyContainer a byte container + * @param message the input buffer + * @param key key used to derive + * @return Fr output field element as uint512_t( H(10...0 || HMAC(k,m)) || H(00...0 || HMAC(k,m)) ) % r */ template Fr get_unbiased_field_from_hmac(const MessageContainer& message, const KeyContainer& key) + requires(Hash::OUTPUT_SIZE == 32) { - auto input = hmac(message, key); + // Strong assumption that works for now with our suite of Hashers + static_assert(Hash::BLOCK_SIZE > Hash::OUTPUT_SIZE); + constexpr size_t DOMAIN_SEPARATOR_SIZE = Hash::BLOCK_SIZE - Hash::OUTPUT_SIZE; - std::vector lo_buffer(input.begin(), input.end()); - lo_buffer.push_back(0); - std::vector hi_buffer(input.begin(), input.end()); - hi_buffer.push_back(1); + // Domain separators whose size ensures we hash a block of the exact size expected by + // the Hasher. + constexpr std::array KLO_DOMAIN_SEPARATOR{}; + constexpr std::array KHI_DOMAIN_SEPARATOR{ 0x1 }; + auto input = hmac(message, key); + + // klo = H(00...0 || input) + std::vector lo_buffer(KLO_DOMAIN_SEPARATOR.begin(), KLO_DOMAIN_SEPARATOR.end()); + std::copy(input.begin(), input.end(), std::back_inserter(lo_buffer)); auto klo = Hash::hash(lo_buffer); + + // khi = H(10...0 || input) + std::vector hi_buffer(KHI_DOMAIN_SEPARATOR.begin(), KHI_DOMAIN_SEPARATOR.end()); + std::copy(input.begin(), input.end(), std::back_inserter(hi_buffer)); auto khi = Hash::hash(hi_buffer); + // full_buffer = khi || klo std::vector full_buffer(khi.begin(), khi.end()); - for (auto& v : klo) { - full_buffer.push_back(v); - } + std::copy(klo.begin(), klo.end(), std::back_inserter(full_buffer)); - uint512_t field_as_u512; - const uint8_t* ptr = &full_buffer[0]; - numeric::read(ptr, field_as_u512); + auto field_as_u512 = from_buffer(full_buffer); Fr result((field_as_u512 % Fr::modulus).lo); return result; From 2ace6b172abfc3aa34cb60bc87cabd248ce0287a Mon Sep 17 00:00:00 2001 From: adr1anh Date: Thu, 1 Dec 2022 14:20:16 +0000 Subject: [PATCH 08/23] CVF 14,15 --- src/aztec/crypto/schnorr/multisig.hpp | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/aztec/crypto/schnorr/multisig.hpp b/src/aztec/crypto/schnorr/multisig.hpp index da552a7d60d..f67a5812f0b 100644 --- a/src/aztec/crypto/schnorr/multisig.hpp +++ b/src/aztec/crypto/schnorr/multisig.hpp @@ -70,12 +70,7 @@ template cl // for std::sort bool operator<(const RoundOnePublicOutput& other) const { - if (R < other.R) { - return true; - } else if (R == other.R && S < other.S) { - return true; - } - return false; + return (R < other.R || (R == other.R && S < other.S)); } bool operator==(const RoundOnePublicOutput& other) const { return R == other.R && S == other.S; } From ab07ef33a19a92273a91dc2a2f35cbd31eb29aea Mon Sep 17 00:00:00 2001 From: adr1anh Date: Thu, 1 Dec 2022 17:18:12 +0000 Subject: [PATCH 09/23] CVF 16, 17, 18, 19, 21, 23 Co-authored-by: Michael Connor --- src/aztec/crypto/schnorr/multisig.hpp | 88 +++++++++++++++++++-------- 1 file changed, 61 insertions(+), 27 deletions(-) diff --git a/src/aztec/crypto/schnorr/multisig.hpp b/src/aztec/crypto/schnorr/multisig.hpp index f67a5812f0b..c52e68e2214 100644 --- a/src/aztec/crypto/schnorr/multisig.hpp +++ b/src/aztec/crypto/schnorr/multisig.hpp @@ -1,7 +1,11 @@ #pragma once +#include +#include +#include #include #include +#include #include "schnorr.hpp" #include "proof_of_possession.hpp" @@ -88,18 +92,19 @@ template cl */ static bool valid_round1_nonces(const std::vector& round1_public_outputs) { - for (auto& [R_user, S_user] : round1_public_outputs) { + for (size_t i = 0; i < round1_public_outputs.size(); ++i) { + auto& [R_user, S_user] = round1_public_outputs[i]; if (!R_user.on_curve() || R_user.is_point_at_infinity()) { - info("Round 1 commitments contains invalid R"); + info("Round 1 commitments contains invalid R at index ", i); return false; } if (!S_user.on_curve() || S_user.is_point_at_infinity()) { - info("Round 1 commitments contains invalid S"); + info("Round 1 commitments contains invalid S at index ", i); return false; } } - if (contains_duplicates(round1_public_outputs)) { - info("Round 1 commitments contains duplicate values"); + if (auto duplicated = duplicated_indices(round1_public_outputs); duplicated.size() > 0) { + info("Round 1 commitments contains duplicate values at indices ", duplicated); return false; } return true; @@ -117,8 +122,8 @@ template cl * @param message * @param aggregate_pubkey the output of `combine_signer_pubkeys` * @param round_1_nonces the public outputs of round 1 from all signers - * @return Fr the nonce challenge `a = int(H_non(G, X_agg, "m_start", m, "m_end" {(R1, S1), ..., (Rn, Sn)})) % r ` - * where r is the field order + * @return Fr the nonce challenge `a = int(H_non(G, X_agg, "m_start", m.size(), m, "m_end" {(R1, S1), ..., (Rn, + * Sn)})) % r ` where r is the field order */ static Fr generate_nonce_challenge(const std::string& message, const affine_element& aggregate_pubkey, @@ -127,7 +132,8 @@ template cl // Domain separation for H_non const std::string domain_separator_nonce("h_nonce"); - // compute nonce challenge H('domain_separator_nonce', G, X, "m_start", m, "m_end", {(R1, S1), ..., (Rn, Sn)}) + // compute nonce challenge + // H('domain_separator_nonce', G, X, "m_start", m.size(), m, "m_end", {(R1, S1), ..., (Rn, Sn)}) std::vector nonce_challenge_buffer; // write domain separator std::copy( @@ -145,6 +151,8 @@ template cl // write "m_start" const std::string m_start = "m_start"; + // write m.size() + write(nonce_challenge_buffer, static_cast(message.size())); std::copy(m_start.begin(), m_start.end(), std::back_inserter(nonce_challenge_buffer)); // write message std::copy(message.begin(), message.end(), std::back_inserter(nonce_challenge_buffer)); @@ -175,23 +183,48 @@ template cl */ static affine_element construct_multisig_nonce(const Fr& a, const std::vector& round_1_nonces) { - element R_sum = G1::point_at_infinity; - element S_sum = G1::point_at_infinity; - for (const auto& nonce : round_1_nonces) { - R_sum += nonce.R; - S_sum += nonce.S; + element R_sum = round_1_nonces[0].R; + element S_sum = round_1_nonces[0].S; + for (size_t i = 1; i < round_1_nonces.size(); ++i) { + const auto& [R, S] = round_1_nonces[i]; + R_sum += R; + S_sum += S; } affine_element R(R_sum + S_sum * a); return R; } - template static bool contains_duplicates(const std::vector& input) + /** + * @brief Returns a vector of indices of elements in input that are included more than once. + * + * @warning The returned list may include an index more than once. + * + * @tparam T implements operator< + * @param input list of elements possibly containing duplicates + * @return std::vector a list of indices of input which are included more than once + */ + template static std::vector duplicated_indices(const std::vector& input) { - std::vector copy(input.begin(), input.end()); - std::sort(copy.begin(), copy.end()); - auto it = std::unique(copy.begin(), copy.end()); - bool wasUnique = (it == copy.end()); - return !wasUnique; + const size_t num_inputs = input.size(); + // indices = [0,1,..., num_inputs-1] + std::vector indices(num_inputs); + std::iota(indices.begin(), indices.end(), 0); + + // sort indices according to input. + // input[indices[i-1]] <= input[indices[i]] + std::sort(indices.begin(), indices.end(), [&](size_t a, size_t b) { return input[a] < input[b]; }); + + // This loop will include multiple copies of the same index if an element appears more than twice. + std::vector duplicates; + for (size_t i = 1; i < num_inputs; ++i) { + const size_t idx1 = indices[i - 1]; + const size_t idx2 = indices[i]; + if (input[idx1] == input[idx2]) { + duplicates.push_back(idx1); + duplicates.push_back(idx2); + } + } + return duplicates; } public: @@ -211,23 +244,28 @@ template cl points.push_back(public_key); } - if (contains_duplicates(points)) { + if (auto duplicated = duplicated_indices(points); duplicated.size() > 0) { + info("Duplicated public keys at indices ", duplicated); return std::nullopt; } element aggregate_pubkey_jac = G1::point_at_infinity; - for (const auto& [public_key, proof_of_possession] : signer_pubkeys) { + for (size_t i = 0; i < signer_pubkeys.size(); ++i) { + const auto& [public_key, proof_of_possession] = signer_pubkeys[i]; if (!public_key.on_curve() || public_key.is_point_at_infinity()) { - info("Multisig signer pubkey not a valid point"); + info("Multisig signer pubkey not a valid point at index ", i); return std::nullopt; } if (!proof_of_possession.verify(public_key)) { - info("Multisig proof of posession invalid"); + info("Multisig proof of posession invalid at index ", i); return std::nullopt; } aggregate_pubkey_jac += public_key; } + // This would prevent accidentally creating an aggregate key for the point at inifinity, + // with the trivial secret key. + // While it shouldn't happen, it is a cheap check. affine_element aggregate_pubkey(aggregate_pubkey_jac); if (aggregate_pubkey.is_point_at_infinity()) { info("Multisig aggregate public key is invalid"); @@ -343,10 +381,6 @@ template cl if (!valid_round1_nonces(round_1_nonces)) { return std::nullopt; } - if (contains_duplicates(round_2_signature_shares)) { - info("Multisig signature shares contains duplicate values"); - return std::nullopt; - } // compute aggregate key X = X_1 + ... + X_n auto aggregate_pubkey = validate_and_combine_signer_pubkeys(signer_pubkeys); From 1557ce1c8d80bd8d5ad93a2ef8074b6c202973da Mon Sep 17 00:00:00 2001 From: adr1anh Date: Thu, 1 Dec 2022 17:26:41 +0000 Subject: [PATCH 10/23] CVF 25, 27, 28, 29 --- src/aztec/crypto/schnorr/proof_of_possession.hpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/aztec/crypto/schnorr/proof_of_possession.hpp b/src/aztec/crypto/schnorr/proof_of_possession.hpp index a0b83c17a21..e66bcac842b 100644 --- a/src/aztec/crypto/schnorr/proof_of_possession.hpp +++ b/src/aztec/crypto/schnorr/proof_of_possession.hpp @@ -39,7 +39,7 @@ template struct ProofOfPossession { auto secret_key = account.private_key; auto public_key = account.public_key; - // use HMAC in PRF mode to derive 32-byte secret `k` + // use HMAC in PRF mode to derive uniformly random nonce `k` from the secret/public key. auto hmac_key = to_buffer(secret_key); auto hmac_message = to_buffer(public_key); Fr k = crypto::get_unbiased_field_from_hmac(hmac_message, hmac_key); @@ -63,7 +63,7 @@ template struct ProofOfPossession { { Fr challenge_fr = Fr::serialize_from_buffer(&challenge[0]); // this ensures that a default constructed proof is invalid - if (challenge_fr.is_zero() || response.is_zero()) + if (response.is_zero()) return false; if (!public_key.on_curve() || public_key.is_point_at_infinity()) @@ -81,11 +81,11 @@ template struct ProofOfPossession { private: /** - * @brief Generate the Fiat-Shamir challenge e = H_reg(G,X,X,R) + * @brief Generate the Fiat-Shamir challenge e = H_reg(G,X,R) * * @param public_key X = secret_key•G * @param R the commitment R = k•G - * @return e = H_reg(X,X,R) + * @return e = H_reg(X,R) */ static auto generate_challenge(const affine_element& public_key, const affine_element& R) { @@ -101,8 +101,7 @@ template struct ProofOfPossession { // write the group generator write(challenge_buf, G1::affine_one); - // write X twice as per the spec - write(challenge_buf, public_key); + // write X (only once, differing from the paper) write(challenge_buf, public_key); // write R From 7e0445969a1f9d7736fd4f48222ecb677b0734bd Mon Sep 17 00:00:00 2001 From: adr1anh Date: Thu, 1 Dec 2022 17:40:44 +0000 Subject: [PATCH 11/23] CVF 34, 36, 39, 41, 43 --- src/aztec/crypto/schnorr/schnorr.hpp | 15 ++------------- src/aztec/crypto/schnorr/schnorr.tcc | 14 ++++++++------ 2 files changed, 10 insertions(+), 19 deletions(-) diff --git a/src/aztec/crypto/schnorr/schnorr.hpp b/src/aztec/crypto/schnorr/schnorr.hpp index f6fe82132fc..27233ea94f6 100644 --- a/src/aztec/crypto/schnorr/schnorr.hpp +++ b/src/aztec/crypto/schnorr/schnorr.hpp @@ -19,13 +19,8 @@ template struct key_pair { }; struct signature { - std::array s; - std::array e; -}; - -struct signature_b { - std::array s; - std::array r; + std::array s; // Fr + std::array e; // Fr }; template @@ -34,12 +29,6 @@ bool verify_signature(const std::string& message, const typename G1::affine_elem template signature construct_signature(const std::string& message, const key_pair& account); -template -signature_b construct_signature_b(const std::string& message, const key_pair& account); - -template -typename G1::affine_element ecrecover(const std::string& message, const signature_b& sig); - inline bool operator==(signature const& lhs, signature const& rhs) { return lhs.s == rhs.s && lhs.e == rhs.e; diff --git a/src/aztec/crypto/schnorr/schnorr.tcc b/src/aztec/crypto/schnorr/schnorr.tcc index 47a4de44bb6..fbc0ea3d9d0 100644 --- a/src/aztec/crypto/schnorr/schnorr.tcc +++ b/src/aztec/crypto/schnorr/schnorr.tcc @@ -11,13 +11,13 @@ namespace schnorr { /** * @brief Generate the schnorr signature challenge parameter `e` given a message, signer pubkey and nonce * - * @details Normal Schnorr param e = H(r.x || pub_key || message) + * @details Normal Schnorr param e = H(R.x || pubkey || message) * But we want to keep hash preimage to <= 64 bytes for a 32 byte message * (for performance reasons in our join-split circuit!) * * barretenberg schnorr defines e as the following: * - * e = H(pedersen(r.x || pub_key.x || pub_key.y), message) + * e = H(pedersen(R.x || pubkey.x || pubkey.y), message) * * pedersen is collision resistant => e can be modelled as randomly distributed * as long as H can be modelled as a random oracle @@ -27,7 +27,7 @@ namespace schnorr { * @param message what are we signing over? * @param pubkey the pubkey of the signer * @param R the nonce - * @return e = H(pedersen(r.x || pub_key.x || pub_key.y), message) as a 256-bit integer, + * @return e = H(pedersen(R.x || pubkey.x || pubkey.y), message) as a 256-bit integer, * represented in a container of 32 uint8_t's * * @@ -74,7 +74,8 @@ signature construct_signature(const std::string& message, const key_pair auto& public_key = account.public_key; auto& private_key = account.private_key; - // use HMAC in PRF mode to derive 32-byte secret `k` + // use HMAC in PRF mode to deterministically derive a uniformly distributed nonce `k` + // from the secret key and message. std::vector pkey_buffer; write(pkey_buffer, private_key); Fr k = crypto::get_unbiased_field_from_hmac(message, pkey_buffer); @@ -113,7 +114,7 @@ bool verify_signature(const std::string& message, const typename G1::affine_elem return false; } // Deserializing from a 256-bit buffer will induce a bias on the order of - // 1/(2(256-log(r))) where r is the order of Fr. + // 1/(2(256-log(r))) where r is the order of Fr, since we perform a modular reduction Fr e = Fr::serialize_from_buffer(&sig.e[0]); // reading s in this way always applies the modular reduction, and @@ -129,7 +130,8 @@ bool verify_signature(const std::string& message, const typename G1::affine_elem // R = g^{sig.s} • pub^{sig.e} affine_element R(element(public_key) * e + G1::one * s); if (R.is_point_at_infinity()) { - // this result implies k == 0 + // this result implies k == 0, which would be catastrophic for the prover. + // it is a cheap check that ensures this doesn't happen. return false; } From 3acf87851acee1ebb03f3ee28e3d0ec094f67ad8 Mon Sep 17 00:00:00 2001 From: adr1anh Date: Fri, 2 Dec 2022 11:23:52 +0000 Subject: [PATCH 12/23] - Address Mike's comments - Fix a bug where in c_bind The `if (auto ...` was missing a check to ensure the optonal was valid --- src/aztec/crypto/schnorr/c_bind.cpp | 2 +- src/aztec/crypto/schnorr/multisig.hpp | 8 ++++---- src/aztec/crypto/schnorr/schnorr.hpp | 14 ++++++++++++-- src/aztec/crypto/schnorr/schnorr.tcc | 5 ++++- 4 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/aztec/crypto/schnorr/c_bind.cpp b/src/aztec/crypto/schnorr/c_bind.cpp index 329c91d883a..1a7a62c57c4 100644 --- a/src/aztec/crypto/schnorr/c_bind.cpp +++ b/src/aztec/crypto/schnorr/c_bind.cpp @@ -66,7 +66,7 @@ WASM_EXPORT bool multisig_validate_and_combine_signer_pubkeys(uint8_t const* sig std::vector pubkeys = from_buffer>(signer_pubkey_buf); - if (auto combined_key = multisig::validate_and_combine_signer_pubkeys(pubkeys)) { + if (auto combined_key = multisig::validate_and_combine_signer_pubkeys(pubkeys); combined_key) { write(combined_key_buf, *combined_key); return true; } else { diff --git a/src/aztec/crypto/schnorr/multisig.hpp b/src/aztec/crypto/schnorr/multisig.hpp index c52e68e2214..3b856fad22f 100644 --- a/src/aztec/crypto/schnorr/multisig.hpp +++ b/src/aztec/crypto/schnorr/multisig.hpp @@ -74,10 +74,10 @@ template cl // for std::sort bool operator<(const RoundOnePublicOutput& other) const { - return (R < other.R || (R == other.R && S < other.S)); + return ((R < other.R) || ((R == other.R) && S < (other.S))); } - bool operator==(const RoundOnePublicOutput& other) const { return R == other.R && S == other.S; } + bool operator==(const RoundOnePublicOutput& other) const { return (R == other.R) && (S == other.S); } }; // corresponds to z = r + as - ex, using RoundTwoPublicOutput = Fr; @@ -147,13 +147,13 @@ template cl // we slightly deviate from the protocol when including 'm', since the length of 'm' is variable // by writing a prefix and a suffix, we prevent the message from being interpreted as coming from a different - // session + // session. // write "m_start" const std::string m_start = "m_start"; + std::copy(m_start.begin(), m_start.end(), std::back_inserter(nonce_challenge_buffer)); // write m.size() write(nonce_challenge_buffer, static_cast(message.size())); - std::copy(m_start.begin(), m_start.end(), std::back_inserter(nonce_challenge_buffer)); // write message std::copy(message.begin(), message.end(), std::back_inserter(nonce_challenge_buffer)); // write "m_end" diff --git a/src/aztec/crypto/schnorr/schnorr.hpp b/src/aztec/crypto/schnorr/schnorr.hpp index 27233ea94f6..a105a4cb4a0 100644 --- a/src/aztec/crypto/schnorr/schnorr.hpp +++ b/src/aztec/crypto/schnorr/schnorr.hpp @@ -18,9 +18,19 @@ template struct key_pair { typename G1::affine_element public_key; }; +// Raw representation of a Schnorr signature (e,s). We use the short variant of Schnorr +// where we include the challenge hash `e` instead of the group element R representing +// the provers initial message. struct signature { - std::array s; // Fr - std::array e; // Fr + + // `s` is a serialized field element (also 32 bytes), representing the prover's response to + // to the verifier challenge `e`. + // We do not enforce that `s` is canonical since signatures are verified inside a circuit, + // and are provided as private inputs. Malleability is not an issue in this case. + std::array s; + // `e` represents the verifier's challenge in the protocol. It is encoded as the 32-byte + // output of a hash function modeling a random oracle in the Fiat-Shamir transform. + std::array e; }; template diff --git a/src/aztec/crypto/schnorr/schnorr.tcc b/src/aztec/crypto/schnorr/schnorr.tcc index fbc0ea3d9d0..ce6a706adb6 100644 --- a/src/aztec/crypto/schnorr/schnorr.tcc +++ b/src/aztec/crypto/schnorr/schnorr.tcc @@ -71,6 +71,10 @@ static auto generate_schnorr_challenge(const std::string& message, template signature construct_signature(const std::string& message, const key_pair& account) { + // sanity check to ensure our hash function produces `e_raw` + // of exactly 32 bytes. + static_assert(Hash::OUTPUT_SIZE == 32); + auto& public_key = account.public_key; auto& private_key = account.private_key; @@ -82,7 +86,6 @@ signature construct_signature(const std::string& message, const key_pair typename G1::affine_element R(G1::one * k); - // container with 32 bytes auto e_raw = generate_schnorr_challenge(message, public_key, R); // the conversion from e_raw results in a biased field element e Fr e = Fr::serialize_from_buffer(&e_raw[0]); From bab6a65d84eef40daa501233072b52d2e30864fc Mon Sep 17 00:00:00 2001 From: adr1anh Date: Fri, 2 Dec 2022 11:29:42 +0000 Subject: [PATCH 13/23] Fixed comment --- src/aztec/crypto/schnorr/proof_of_possession.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/aztec/crypto/schnorr/proof_of_possession.hpp b/src/aztec/crypto/schnorr/proof_of_possession.hpp index e66bcac842b..f336a385de5 100644 --- a/src/aztec/crypto/schnorr/proof_of_possession.hpp +++ b/src/aztec/crypto/schnorr/proof_of_possession.hpp @@ -22,7 +22,7 @@ template struct ProofOfPossession { using element = typename G1::element; using key_pair = crypto::schnorr::key_pair; - // challenge = e = H_reg(pk,pk,R) + // challenge = e = H_reg(pk,R) std::array challenge; // response = z = k - e * sk Fr response = Fr::zero(); @@ -92,7 +92,7 @@ template struct ProofOfPossession { // Domain separation challenges const std::string domain_separator_pop("h_reg"); - // buffer containing (domain_sep, G, X, X, R) + // buffer containing (domain_sep, G, X, R) std::vector challenge_buf; // write domain separator @@ -107,7 +107,7 @@ template struct ProofOfPossession { // write R write(challenge_buf, R); - // generate the raw bits of H_reg(X,X,R) + // generate the raw bits of H_reg(X,R) return Hash::hash(challenge_buf); } }; From 650282fc47c0405bc69d9e6c95fe89a7201d07e6 Mon Sep 17 00:00:00 2001 From: adr1anh Date: Sat, 3 Dec 2022 10:47:05 +0000 Subject: [PATCH 14/23] Replace HMAC nonce derivation with pure randomness --- src/aztec/crypto/schnorr/proof_of_possession.hpp | 10 +++++----- src/aztec/crypto/schnorr/schnorr.tcc | 9 ++++----- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/aztec/crypto/schnorr/proof_of_possession.hpp b/src/aztec/crypto/schnorr/proof_of_possession.hpp index f336a385de5..69430a60999 100644 --- a/src/aztec/crypto/schnorr/proof_of_possession.hpp +++ b/src/aztec/crypto/schnorr/proof_of_possession.hpp @@ -29,8 +29,11 @@ template struct ProofOfPossession { // restore default constructor to enable deserialization ProofOfPossession() = default; + /** - * @brief Deterministically create a proof of possession for the given account + * @brief Create a new proof of possession for a given account. + * + * @warning Proofs are not deterministic. * * @param account a key_pair (secret_key, public_key) */ @@ -39,10 +42,7 @@ template struct ProofOfPossession { auto secret_key = account.private_key; auto public_key = account.public_key; - // use HMAC in PRF mode to derive uniformly random nonce `k` from the secret/public key. - auto hmac_key = to_buffer(secret_key); - auto hmac_message = to_buffer(public_key); - Fr k = crypto::get_unbiased_field_from_hmac(hmac_message, hmac_key); + Fr k = Fr::random_element(); affine_element R = G1::one * k; diff --git a/src/aztec/crypto/schnorr/schnorr.tcc b/src/aztec/crypto/schnorr/schnorr.tcc index ce6a706adb6..923a9dfdd78 100644 --- a/src/aztec/crypto/schnorr/schnorr.tcc +++ b/src/aztec/crypto/schnorr/schnorr.tcc @@ -57,6 +57,8 @@ static auto generate_schnorr_challenge(const std::string& message, /** * @brief Construct a Schnorr signature of the form (random - priv * hash, hash) using the group G1. * + * @warning Proofs are not deterministic. + * * @tparam Hash: A function std::vector -> std::array * @tparam Fq: The field over which points of G1 are defined. * @tparam Fr: A class with a random element generator, where the multiplication @@ -78,11 +80,8 @@ signature construct_signature(const std::string& message, const key_pair auto& public_key = account.public_key; auto& private_key = account.private_key; - // use HMAC in PRF mode to deterministically derive a uniformly distributed nonce `k` - // from the secret key and message. - std::vector pkey_buffer; - write(pkey_buffer, private_key); - Fr k = crypto::get_unbiased_field_from_hmac(message, pkey_buffer); + // sample random nonce k + Fr k = Fr::random_element(); typename G1::affine_element R(G1::one * k); From e5022607f24d4c1f7c24f4febd1f8d9f1c6fe914 Mon Sep 17 00:00:00 2001 From: adr1anh Date: Mon, 5 Dec 2022 09:48:24 +0000 Subject: [PATCH 15/23] revert PoP challenge optimization add comments about zero-ing variables add comment about random sampling --- src/aztec/crypto/hmac/hmac.hpp | 7 +++++++ src/aztec/crypto/schnorr/multisig.hpp | 2 ++ .../crypto/schnorr/proof_of_possession.hpp | 20 +++++++++++++------ src/aztec/crypto/schnorr/schnorr.tcc | 9 +++++++++ 4 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/aztec/crypto/hmac/hmac.hpp b/src/aztec/crypto/hmac/hmac.hpp index 337974176c5..972b1016654 100644 --- a/src/aztec/crypto/hmac/hmac.hpp +++ b/src/aztec/crypto/hmac/hmac.hpp @@ -34,6 +34,7 @@ std::array hmac(const MessageContainer& message, con // initialize k_prime to 0x00,...,0x00 // copy key or truncated key to start. + // TODO: securely erase `k_prime` std::array k_prime{}; if (key.size() > B) { const auto truncated_key = Hash::hash(key); @@ -42,22 +43,28 @@ std::array hmac(const MessageContainer& message, con std::copy(key.begin(), key.end(), k_prime.begin()); } + // TODO: securely erase `h1` std::array h1; for (size_t i = 0; i < B; ++i) { h1[i] = k_prime[i] ^ opad[i]; } + // TODO: securely erase `h2` std::array h2; for (size_t i = 0; i < B; ++i) { h2[i] = k_prime[i] ^ ipad[i]; } + // TODO: securely erase copy of `h2` in `message_buffer`, + // ensure `message_buffer` is not re-allocated std::vector message_buffer; std::copy(h2.begin(), h2.end(), std::back_inserter(message_buffer)); std::copy(message.begin(), message.end(), std::back_inserter(message_buffer)); const auto h3 = Hash::hash(message_buffer); + // TODO: securely erase copy of `h1` in `hmac_buffer`, + // ensure `hmac_buffer` is not re-allocated std::vector hmac_buffer; std::copy(h1.begin(), h1.end(), std::back_inserter(hmac_buffer)); std::copy(h3.begin(), h3.end(), std::back_inserter(hmac_buffer)); diff --git a/src/aztec/crypto/schnorr/multisig.hpp b/src/aztec/crypto/schnorr/multisig.hpp index 3b856fad22f..aa94a347b86 100644 --- a/src/aztec/crypto/schnorr/multisig.hpp +++ b/src/aztec/crypto/schnorr/multisig.hpp @@ -285,11 +285,13 @@ template cl static std::pair construct_signature_round_1() { // r_user ← 𝔽 + // TODO: securely erase `r_user` Fr r_user = Fr::random_element(); // R_user ← r_user⋅G affine_element R_user = G1::one * r_user; // s_user ← 𝔽 + // TODO: securely erase `s_user` Fr s_user = Fr::random_element(); // S_user ← s_user⋅G affine_element S_user = G1::one * s_user; diff --git a/src/aztec/crypto/schnorr/proof_of_possession.hpp b/src/aztec/crypto/schnorr/proof_of_possession.hpp index 69430a60999..4e27c6472bd 100644 --- a/src/aztec/crypto/schnorr/proof_of_possession.hpp +++ b/src/aztec/crypto/schnorr/proof_of_possession.hpp @@ -22,7 +22,7 @@ template struct ProofOfPossession { using element = typename G1::element; using key_pair = crypto::schnorr::key_pair; - // challenge = e = H_reg(pk,R) + // challenge = e = H_reg(pk,pk,R) std::array challenge; // response = z = k - e * sk Fr response = Fr::zero(); @@ -42,6 +42,13 @@ template struct ProofOfPossession { auto secret_key = account.private_key; auto public_key = account.public_key; + // Fr::random_element() will call std::random_device, which in turn relies on system calls to generate a string + // of random bits. It is important to ensure that the execution environment will correctly supply system calls + // that give std::random_device access to an entropy source that produces a string of non-deterministic + // uniformly random bits. For example, when compiling into a wasm binary, it is essential that the random_get + // method is overloaded to utilise a suitable entropy source + // (see https://github.com/WebAssembly/WASI/blob/main/phases/snapshot/docs.md) + // TODO: securely erase `k` Fr k = Fr::random_element(); affine_element R = G1::one * k; @@ -81,18 +88,18 @@ template struct ProofOfPossession { private: /** - * @brief Generate the Fiat-Shamir challenge e = H_reg(G,X,R) + * @brief Generate the Fiat-Shamir challenge e = H_reg(G,X,X,R) * * @param public_key X = secret_key•G * @param R the commitment R = k•G - * @return e = H_reg(X,R) + * @return e = H_reg(X,X,R) */ static auto generate_challenge(const affine_element& public_key, const affine_element& R) { // Domain separation challenges const std::string domain_separator_pop("h_reg"); - // buffer containing (domain_sep, G, X, R) + // buffer containing (domain_sep, G, X, X, R) std::vector challenge_buf; // write domain separator @@ -101,13 +108,14 @@ template struct ProofOfPossession { // write the group generator write(challenge_buf, G1::affine_one); - // write X (only once, differing from the paper) + // write X twice as per the spec + write(challenge_buf, public_key); write(challenge_buf, public_key); // write R write(challenge_buf, R); - // generate the raw bits of H_reg(X,R) + // generate the raw bits of H_reg(X,X,R) return Hash::hash(challenge_buf); } }; diff --git a/src/aztec/crypto/schnorr/schnorr.tcc b/src/aztec/crypto/schnorr/schnorr.tcc index 923a9dfdd78..fe2eae44560 100644 --- a/src/aztec/crypto/schnorr/schnorr.tcc +++ b/src/aztec/crypto/schnorr/schnorr.tcc @@ -81,6 +81,15 @@ signature construct_signature(const std::string& message, const key_pair auto& private_key = account.private_key; // sample random nonce k + // + // Fr::random_element() will call std::random_device, which in turn relies on system calls to generate a string + // of random bits. It is important to ensure that the execution environment will correctly supply system calls + // that give std::random_device access to an entropy source that produces a string of non-deterministic + // uniformly random bits. For example, when compiling into a wasm binary, it is essential that the random_get + // method is overloaded to utilise a suitable entropy source + // (see https://github.com/WebAssembly/WASI/blob/main/phases/snapshot/docs.md) + // + // TODO: securely erase `k` Fr k = Fr::random_element(); typename G1::affine_element R(G1::one * k); From 98e05a13b5c9332639f153d448203171df2d6c18 Mon Sep 17 00:00:00 2001 From: Innokentii Sennovskii Date: Mon, 5 Dec 2022 15:28:38 +0000 Subject: [PATCH 16/23] Changing logs back (#1865) * Change logs back and disable defi-bridge-project filter * Switch filtering back to defi-bridge --- .../rollup/proofs/compute_circuit_data.hpp | 70 +++++++++++++------ .../proofs/rollup/compute_circuit_data.hpp | 27 +++---- .../root_rollup/compute_circuit_data.cpp | 26 +++---- .../root_verifier/compute_circuit_data.hpp | 5 +- 4 files changed, 79 insertions(+), 49 deletions(-) diff --git a/src/aztec/rollup/proofs/compute_circuit_data.hpp b/src/aztec/rollup/proofs/compute_circuit_data.hpp index 882b6b35ead..ed9c2edbc15 100644 --- a/src/aztec/rollup/proofs/compute_circuit_data.hpp +++ b/src/aztec/rollup/proofs/compute_circuit_data.hpp @@ -49,7 +49,8 @@ circuit_data get_circuit_data(std::string const& name, bool vk, bool padding, bool mock, - F const& build_circuit) + F const& build_circuit, + std::string const name_suffix_for_benchmarks = "") { circuit_data data; data.srs = srs; @@ -71,18 +72,27 @@ circuit_data get_circuit_data(std::string const& name, Timer timer; build_circuit(composer); - benchmark_collator.benchmark_info_deferred( - GET_COMPOSER_NAME_STRING(ComposerType), "Core", name, "Build time", timer.toString()); - benchmark_collator.benchmark_info_deferred( - GET_COMPOSER_NAME_STRING(ComposerType), "Core", name, "Gates", composer.get_num_gates()); + benchmark_collator.benchmark_info_deferred(GET_COMPOSER_NAME_STRING(ComposerType), + "Core", + name + name_suffix_for_benchmarks, + "Build time", + timer.toString()); + benchmark_collator.benchmark_info_deferred(GET_COMPOSER_NAME_STRING(ComposerType), + "Core", + name + name_suffix_for_benchmarks, + "Gates", + composer.get_num_gates()); info(name, ": Circuit built in: ", timer.toString(), "s"); info(name, ": Circuit size: ", composer.get_num_gates()); if (mock) { auto public_inputs = composer.get_public_inputs(); mock::mock_circuit(mock_proof_composer, public_inputs); info(name, ": Mock circuit size: ", mock_proof_composer.get_num_gates()); - benchmark_collator.benchmark_info_deferred( - GET_COMPOSER_NAME_STRING(ComposerType), "Core", name, "Mock Gates", composer.get_num_gates()); + benchmark_collator.benchmark_info_deferred(GET_COMPOSER_NAME_STRING(ComposerType), + "Core", + name + name_suffix_for_benchmarks, + "Mock Gates", + composer.get_num_gates()); } } @@ -103,8 +113,11 @@ circuit_data get_circuit_data(std::string const& name, std::make_shared(std::move(pk_data), srs->get_prover_crs(pk_data.n + 1)); data.num_gates = pk_data.n; info(name, ": Circuit size 2^n: ", data.num_gates); - benchmark_collator.benchmark_info_deferred( - GET_COMPOSER_NAME_STRING(ComposerType), "Core", name, "Gates 2^n", data.num_gates); + benchmark_collator.benchmark_info_deferred(GET_COMPOSER_NAME_STRING(ComposerType), + "Core", + name + name_suffix_for_benchmarks, + "Gates 2^n", + data.num_gates); } else if (compute) { Timer timer; info(name, ": Computing proving key..."); @@ -114,20 +127,29 @@ circuit_data get_circuit_data(std::string const& name, data.proving_key = composer.compute_proving_key(); info(name, ": Circuit size 2^n: ", data.proving_key->n); - benchmark_collator.benchmark_info_deferred( - GET_COMPOSER_NAME_STRING(ComposerType), "Core", name, "Gates 2^n", data.proving_key->n); + benchmark_collator.benchmark_info_deferred(GET_COMPOSER_NAME_STRING(ComposerType), + "Core", + name + name_suffix_for_benchmarks, + "Gates 2^n", + data.proving_key->n); } else { data.num_gates = mock_proof_composer.get_num_gates(); data.proving_key = mock_proof_composer.compute_proving_key(); info(name, ": Mock circuit size 2^n: ", data.proving_key->n); - benchmark_collator.benchmark_info_deferred( - GET_COMPOSER_NAME_STRING(ComposerType), "Core", name, "Mock Gates 2^n", data.proving_key->n); + benchmark_collator.benchmark_info_deferred(GET_COMPOSER_NAME_STRING(ComposerType), + "Core", + name + name_suffix_for_benchmarks, + "Mock Gates 2^n", + data.proving_key->n); } info(name, ": Proving key computed in ", timer.toString(), "s"); - benchmark_collator.benchmark_info_deferred( - GET_COMPOSER_NAME_STRING(ComposerType), "Core", name, "Proving key computed in", timer.toString()); + benchmark_collator.benchmark_info_deferred(GET_COMPOSER_NAME_STRING(ComposerType), + "Core", + name + name_suffix_for_benchmarks, + "Proving key computed in", + timer.toString()); if (save) { info(name, ": Saving proving key..."); std::filesystem::create_directories(pk_dir.c_str()); @@ -153,7 +175,7 @@ circuit_data get_circuit_data(std::string const& name, info(name, ": Verification key hash: ", data.verification_key->sha256_hash()); benchmark_collator.benchmark_info_deferred(GET_COMPOSER_NAME_STRING(ComposerType), "Core", - name, + name + name_suffix_for_benchmarks, "Verification key hash", data.verification_key->sha256_hash()); } else if (compute) { @@ -167,12 +189,15 @@ circuit_data get_circuit_data(std::string const& name, } info(name, ": Computed verification key in ", timer.toString(), "s"); - benchmark_collator.benchmark_info_deferred( - GET_COMPOSER_NAME_STRING(ComposerType), "Core", name, "Verification key computed in", timer.toString()); + benchmark_collator.benchmark_info_deferred(GET_COMPOSER_NAME_STRING(ComposerType), + "Core", + name + name_suffix_for_benchmarks, + "Verification key computed in", + timer.toString()); info(name, ": Verification key hash: ", data.verification_key->sha256_hash()); benchmark_collator.benchmark_info_deferred(GET_COMPOSER_NAME_STRING(ComposerType), "Core", - name, + name + name_suffix_for_benchmarks, "Verification key hash", data.verification_key->sha256_hash()); @@ -219,8 +244,11 @@ circuit_data get_circuit_data(std::string const& name, info(name, ": Padding verified: ", verifier.verify_proof(proof)); } info(name, ": Padding proof computed in ", timer.toString(), "s"); - benchmark_collator.benchmark_info_deferred( - GET_COMPOSER_NAME_STRING(ComposerType), "Core", name, "Padding proof computed in", timer.toString()); + benchmark_collator.benchmark_info_deferred(GET_COMPOSER_NAME_STRING(ComposerType), + "Core", + name + name_suffix_for_benchmarks, + "Padding proof computed in", + timer.toString()); if (save) { std::ofstream os(padding_path); diff --git a/src/aztec/rollup/proofs/rollup/compute_circuit_data.hpp b/src/aztec/rollup/proofs/rollup/compute_circuit_data.hpp index ea72e9f3d2a..e0b2e9013fc 100644 --- a/src/aztec/rollup/proofs/rollup/compute_circuit_data.hpp +++ b/src/aztec/rollup/proofs/rollup/compute_circuit_data.hpp @@ -50,19 +50,20 @@ inline circuit_data get_circuit_data(size_t rollup_size, rollup_circuit(composer, rollup, verification_keys, rollup_size); }; - auto cd = proofs::get_circuit_data("tx rollup " + std::to_string(rollup_size) + "x" + - std::to_string(rollup_size_pow2), - name, - srs, - key_path, - compute, - save, - load, - pk, - vk, - true, - mock, - build_circuit); + auto cd = + proofs::get_circuit_data("tx rollup", + name, + srs, + key_path, + compute, + save, + load, + pk, + vk, + true, + mock, + build_circuit, + " " + std::to_string(rollup_size) + "x" + std::to_string(rollup_size_pow2)); circuit_data data; data.num_gates = cd.num_gates; diff --git a/src/aztec/rollup/proofs/root_rollup/compute_circuit_data.cpp b/src/aztec/rollup/proofs/root_rollup/compute_circuit_data.cpp index becfe588272..b7904889952 100644 --- a/src/aztec/rollup/proofs/root_rollup/compute_circuit_data.cpp +++ b/src/aztec/rollup/proofs/root_rollup/compute_circuit_data.cpp @@ -55,19 +55,19 @@ circuit_data get_circuit_data(size_t num_inner_rollups, rollup_circuit_data.verification_key); }; - auto cd = - proofs::get_circuit_data(format("root rollup ", rollup_circuit_data.num_txs, "x", num_inner_rollups), - name, - srs, - key_path, - compute, - save, - load, - pk, - vk, - true, - mock, - build_circuit); + auto cd = proofs::get_circuit_data("root rollup", + name, + srs, + key_path, + compute, + save, + load, + pk, + vk, + true, + mock, + build_circuit, + format(" ", rollup_circuit_data.num_txs, "x", num_inner_rollups)); circuit_data data; data.num_gates = cd.num_gates; diff --git a/src/aztec/rollup/proofs/root_verifier/compute_circuit_data.hpp b/src/aztec/rollup/proofs/root_verifier/compute_circuit_data.hpp index ac330f6f9ca..7c3056ef23e 100644 --- a/src/aztec/rollup/proofs/root_verifier/compute_circuit_data.hpp +++ b/src/aztec/rollup/proofs/root_verifier/compute_circuit_data.hpp @@ -34,7 +34,7 @@ inline circuit_data get_circuit_data(root_rollup::circuit_data const& root_rollu auto cd = proofs::get_circuit_data( - format("root verifier ", root_rollup_circuit_data.inner_rollup_circuit_data.rollup_size, "x", valid_vks.size()), + "root verifier", name, srs, key_path, @@ -45,7 +45,8 @@ inline circuit_data get_circuit_data(root_rollup::circuit_data const& root_rollu vk, false, mock, - build_verifier_circuit); + build_verifier_circuit, + format(" ", root_rollup_circuit_data.inner_rollup_circuit_data.rollup_size, "x", valid_vks.size())); circuit_data data; data.num_gates = cd.num_gates; From 0bfdfb6e97a1b8f164ea64e380558a5b89838264 Mon Sep 17 00:00:00 2001 From: Innokentii Sennovskii Date: Mon, 5 Dec 2022 16:49:26 +0000 Subject: [PATCH 17/23] Fuzzing mode + bigfield, safe_uint and field fuzzers take 2 (#1787) * Enabled fuzzer mode * Added safe_uint fuzzer * Optimized check_circuit method in turbo_composer for more efficient fuzzing * Added fixes to circumvent assert checks when fuzzing with asserts * Added bigfield fuzzer * Added assert circumvention to bigfield fuzzer * Add fuzzer enhancements * Fixing stuff broken by low-memory prover changes * Documentation changes * Add byte_array fuzzer * Add bit_array fuzzer * bit_array fuzzer: Support the SLICE operation * bit_array fuzzer: Support the SET operation * byte_array fuzzer: Support the SET operation * field_t fuzzer: Support the SET operation * byte_array fuzzer: Invoke safe_uint_t-based constructor in SET operation * byte_array fuzzer: Fix invocation of safe_uint_t-based constructor * bigfield_t fuzzer: Support the SET operation * field_t fuzzer: Call additional field_t functions - invert() - accumulate() - assert_is_in_set() - decompose_into_bits() - operator bool_t() * bigfield_t fuzzer: Additional conversions in the SET operation * byte_array fuzzer: Invoke field_t constructor with variable byte size * Add uint fuzzer * uint fuzzer: Test all types (u8, u16, u32, u64) simultaneously * uint fuzzer: Implement the NOT opcode * uint fuzzer: Ensure uint256_t returned by get_value() has appropriate width * uint fuzzer: Add explicit sanity checks around byte_array_t/field_t conversions * uint fuzzer: Make methods static or const were appropriate * Build fuzzers with different composers * Add missing file * bigfield fuzzer: Add invariant check * Fix writeInstruction in bit_array fuzzer * Add bool fuzzer * bigfield fuzzer: Fix call to dual field_t constructor * bigfield fuzzer: Avoid divisions by zero * bigfield fuzzer: Fix call to dual field_t constructor * bigfield fuzzer: Multi-numerator division * bigfield fuzzer: Assert bigfield_t context pointer is not NULL * bigfield fuzzer: Fix multi-numerator division * Fixed cpp/hpp issue * Applied Guido's postprocessing fix * Added check_circuit fix for turbocomposer * Slightly updated docs * Disabled check_circuit error printing in Turbo in fuzzing mode * Updated toolchain * Disabling executables in a different way * More cmake optimizations * A few more makelist updates * A bit more * Removed free space Co-authored-by: Guido Vranken --- CMakeLists.txt | 24 + README.md | 16 + cmake/module.cmake | 26 +- cmake/toolchain.cmake | 14 +- docs/Fuzzing.md | 100 + src/aztec/common/fuzzer.hpp | 598 +++++ src/aztec/common/fuzzer_constants.hpp | 7 + src/aztec/plonk/composer/turbo_composer.cpp | 65 +- src/aztec/plonk/composer/turbo_composer.hpp | 2 +- .../turbo_arithmetic_widget.hpp | 14 + .../turbo_fixed_base_widget.hpp | 14 + .../transition_widgets/turbo_logic_widget.hpp | 13 + .../transition_widgets/turbo_range_widget.hpp | 13 + src/aztec/rollup/CMakeLists.txt | 4 +- src/aztec/rollup/proofs/CMakeLists.txt | 2 +- .../primitives/bigfield/bigfield.fuzzer.hpp | 1970 ++++++++++++++++ .../bigfield/bigfield_all.fuzzer.cpp | 3 + .../bigfield/bigfield_standard.fuzzer.cpp | 3 + .../bigfield/bigfield_turbo.fuzzer.cpp | 3 + .../primitives/bit_array/bit_array.fuzzer.hpp | 922 ++++++++ .../bit_array/bit_array_all.fuzzer.cpp | 3 + .../bit_array/bit_array_standard.fuzzer.cpp | 3 + .../bit_array/bit_array_turbo.fuzzer.cpp | 3 + .../stdlib/primitives/bool/bool.fuzzer.hpp | 884 ++++++++ .../primitives/bool/bool_all.fuzzer.cpp | 3 + .../primitives/bool/bool_standard.fuzzer.cpp | 3 + .../primitives/bool/bool_turbo.fuzzer.cpp | 3 + .../byte_array/byte_array.fuzzer.hpp | 970 ++++++++ .../byte_array/byte_array_all.fuzzer.cpp | 3 + .../byte_array/byte_array_standard.fuzzer.cpp | 3 + .../byte_array/byte_array_turbo.fuzzer.cpp | 3 + .../stdlib/primitives/field/field.fuzzer.hpp | 2019 +++++++++++++++++ .../primitives/field/field_all.fuzzer.cpp | 3 + .../field/field_standard.fuzzer.cpp | 3 + .../primitives/field/field_turbo.fuzzer.cpp | 3 + .../primitives/safe_uint/safe_uint.fuzzer.hpp | 1457 ++++++++++++ .../safe_uint/safe_uint_all.fuzzer.cpp | 3 + .../safe_uint/safe_uint_standard.fuzzer.cpp | 3 + .../safe_uint/safe_uint_turbo.fuzzer.cpp | 3 + .../stdlib/primitives/uint/uint.fuzzer.hpp | 1585 +++++++++++++ .../primitives/uint/uint_all.fuzzer.cpp | 3 + .../primitives/uint/uint_standard.fuzzer.cpp | 3 + .../primitives/uint/uint_turbo.fuzzer.cpp | 3 + 43 files changed, 10749 insertions(+), 33 deletions(-) create mode 100644 docs/Fuzzing.md create mode 100644 src/aztec/common/fuzzer.hpp create mode 100644 src/aztec/common/fuzzer_constants.hpp create mode 100644 src/aztec/stdlib/primitives/bigfield/bigfield.fuzzer.hpp create mode 100644 src/aztec/stdlib/primitives/bigfield/bigfield_all.fuzzer.cpp create mode 100644 src/aztec/stdlib/primitives/bigfield/bigfield_standard.fuzzer.cpp create mode 100644 src/aztec/stdlib/primitives/bigfield/bigfield_turbo.fuzzer.cpp create mode 100644 src/aztec/stdlib/primitives/bit_array/bit_array.fuzzer.hpp create mode 100644 src/aztec/stdlib/primitives/bit_array/bit_array_all.fuzzer.cpp create mode 100644 src/aztec/stdlib/primitives/bit_array/bit_array_standard.fuzzer.cpp create mode 100644 src/aztec/stdlib/primitives/bit_array/bit_array_turbo.fuzzer.cpp create mode 100644 src/aztec/stdlib/primitives/bool/bool.fuzzer.hpp create mode 100644 src/aztec/stdlib/primitives/bool/bool_all.fuzzer.cpp create mode 100644 src/aztec/stdlib/primitives/bool/bool_standard.fuzzer.cpp create mode 100644 src/aztec/stdlib/primitives/bool/bool_turbo.fuzzer.cpp create mode 100644 src/aztec/stdlib/primitives/byte_array/byte_array.fuzzer.hpp create mode 100644 src/aztec/stdlib/primitives/byte_array/byte_array_all.fuzzer.cpp create mode 100644 src/aztec/stdlib/primitives/byte_array/byte_array_standard.fuzzer.cpp create mode 100644 src/aztec/stdlib/primitives/byte_array/byte_array_turbo.fuzzer.cpp create mode 100644 src/aztec/stdlib/primitives/field/field.fuzzer.hpp create mode 100644 src/aztec/stdlib/primitives/field/field_all.fuzzer.cpp create mode 100644 src/aztec/stdlib/primitives/field/field_standard.fuzzer.cpp create mode 100644 src/aztec/stdlib/primitives/field/field_turbo.fuzzer.cpp create mode 100644 src/aztec/stdlib/primitives/safe_uint/safe_uint.fuzzer.hpp create mode 100644 src/aztec/stdlib/primitives/safe_uint/safe_uint_all.fuzzer.cpp create mode 100644 src/aztec/stdlib/primitives/safe_uint/safe_uint_standard.fuzzer.cpp create mode 100644 src/aztec/stdlib/primitives/safe_uint/safe_uint_turbo.fuzzer.cpp create mode 100644 src/aztec/stdlib/primitives/uint/uint.fuzzer.hpp create mode 100644 src/aztec/stdlib/primitives/uint/uint_all.fuzzer.cpp create mode 100644 src/aztec/stdlib/primitives/uint/uint_standard.fuzzer.cpp create mode 100644 src/aztec/stdlib/primitives/uint/uint_turbo.fuzzer.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 8c9a883faf5..5805f2db924 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -24,6 +24,30 @@ if(ARM) set(RUN_HAVE_POSIX_REGEX 0) endif() +if(FUZZING) + add_definitions(-DFUZZING=1) + + if(DISABLE_CUSTOM_MUTATORS) + add_definitions(-DDISABLE_CUSTOM_MUTATORS=1) + endif() + + set(SANITIZER_OPTIONS "") + + if(ADDRESS_SANITIZER) + set(SANITIZER_OPTIONS ${SANITIZER_OPTIONS} -fsanitize=address) + endif() + + if(UNDEFINED_BEHAVIOUR_SANITIZER) + set(SANITIZER_OPTIONS ${SANITIZER_OPTIONS} -fsanitize=undefined -fno-sanitize=alignment) + endif() + + add_compile_options(-fsanitize=fuzzer-no-link ${SANITIZER_OPTIONS}) + + set(WASM OFF) + set(BENCHMARKS OFF) + set(TESTING OFF) +endif() + if(WASM) message(STATUS "Compiling for WebAssembly.") set(DISABLE_ASM ON) diff --git a/README.md b/README.md index bb3ac28d2a7..9e478a22eae 100644 --- a/README.md +++ b/README.md @@ -92,6 +92,7 @@ CMake can be passed various build options on it's command line: - `-DTESTING=ON | OFF`: Enable/disable building of tests. - `-DBENCHMARK=ON | OFF`: Enable/disable building of benchmarks. - `-DTOOLCHAIN=`: Use one of the preconfigured toolchains. +- `-DFUZZING=ON | OFF`: Enable building various fuzzers. ### WASM build @@ -117,3 +118,18 @@ Tests can be built and run like: make ecc_tests wasmtime --dir=.. ./bin/ecc_tests ``` + +### Fuzzing build + +To build: +``` +mkdir build-fuzzing && cd build-fuzzing +cmake -DTOOLCHAIN=x86_64-linux-clang -DFUZZING=ON .. +make +``` +Fuzzing build turns off building tests and benchmarks, since they are incompatible with libfuzzer interface. + +To turn on address sanitizer add `-DADDRESS_SANITIZER=ON`. Note that address sanitizer can be used to explore crashes. +Sometimes you might have to specify the address of llvm-symbolizer. You have to do it with `export ASAN_SYMBOLIZER_PATH=`. +For undefined behaviour sanitizer `-DUNDEFINED_BEHAVIOUR_SANITIZER=ON`. +Note that the fuzzer can be orders of magnitude slower with ASan (2-3x slower) or UBSan on, so it is best to run a non-sanitized build first, minimize the testcase and then run it for a bit of time with sanitizers. diff --git a/cmake/module.cmake b/cmake/module.cmake index a4edd3cf9f9..063e3f690ad 100644 --- a/cmake/module.cmake +++ b/cmake/module.cmake @@ -15,7 +15,7 @@ function(barretenberg_module MODULE_NAME) file(GLOB_RECURSE SOURCE_FILES *.cpp) file(GLOB_RECURSE HEADER_FILES *.hpp) - list(FILTER SOURCE_FILES EXCLUDE REGEX ".*\.(test|bench).cpp$") + list(FILTER SOURCE_FILES EXCLUDE REGEX ".*\.(fuzzer|test|bench).cpp$") if(SOURCE_FILES) add_library( @@ -103,6 +103,30 @@ function(barretenberg_module MODULE_NAME) ) endif() + file(GLOB_RECURSE FUZZERS_SOURCE_FILES *.fuzzer.cpp) + if(FUZZING AND FUZZERS_SOURCE_FILES) + foreach(FUZZER_SOURCE_FILE ${FUZZERS_SOURCE_FILES}) + get_filename_component(FUZZER_NAME_STEM ${FUZZER_SOURCE_FILE} NAME_WE) + add_executable( + ${MODULE_NAME}_${FUZZER_NAME_STEM}_fuzzer + ${FUZZER_SOURCE_FILE} + ) + + target_link_options( + ${MODULE_NAME}_${FUZZER_NAME_STEM}_fuzzer + PRIVATE + "-fsanitize=fuzzer" + ${SANITIZER_OPTIONS} + ) + + target_link_libraries( + ${MODULE_NAME}_${FUZZER_NAME_STEM}_fuzzer + PRIVATE + ${MODULE_LINK_NAME} + ) + endforeach() + endif() + file(GLOB_RECURSE BENCH_SOURCE_FILES *.bench.cpp) if(BENCHMARKS AND BENCH_SOURCE_FILES) add_library( diff --git a/cmake/toolchain.cmake b/cmake/toolchain.cmake index 9419a7d199c..4de860fac49 100644 --- a/cmake/toolchain.cmake +++ b/cmake/toolchain.cmake @@ -1,6 +1,10 @@ -if(NOT TOOLCHAIN) - set(TOOLCHAIN "x86_64-linux-clang" CACHE STRING "Build toolchain." FORCE) -endif() -message(STATUS "Toolchain: ${TOOLCHAIN}") +if (CMAKE_C_COMPILER AND CMAKE_CXX_COMPILER) + message(STATUS "Toolchain: manually chosen ${CMAKE_C_COMPILER} and ${CMAKE_CXX_COMPILER}") +else() + if(NOT TOOLCHAIN) + set(TOOLCHAIN "x86_64-linux-clang" CACHE STRING "Build toolchain." FORCE) + endif() + message(STATUS "Toolchain: ${TOOLCHAIN}") -include("./cmake/toolchains/${TOOLCHAIN}.cmake") \ No newline at end of file + include("./cmake/toolchains/${TOOLCHAIN}.cmake") +endif() \ No newline at end of file diff --git a/docs/Fuzzing.md b/docs/Fuzzing.md new file mode 100644 index 00000000000..629e7526ea2 --- /dev/null +++ b/docs/Fuzzing.md @@ -0,0 +1,100 @@ +# Fuzzing barretenberg +## Intro +We are gradually introducing fuzzing of various primitives into barretenberg, focusing first and foremost on in-cicruit types. If you are developing / patching a primitive and there is a fuzzer available for it, please take the time to update the fuzzer (if you've added new functionality) and run it for at least a few hours to increase security. + +## Build + +To build with standard clang: +``` +mkdir build-fuzzing && cd build-fuzzing +cmake -DFUZZING=ON .. +make +``` +Fuzzing build turns off building tests and benchmarks, since they are incompatible with libfuzzer interface. + +To turn on address sanitizer add `-DADDRESS_SANITIZER=ON`. Note that address sanitizer can be used to explore crashes. +Sometimes you might have to specify the address of llvm-symbolizer. You have to do it with `export ASAN_SYMBOLIZER_PATH=`. +For undefined behaviour sanitizer `-DUNDEFINED_BEHAVIOUR_SANITIZER=ON`. +Note that the fuzzer can be orders of magnitude slower with ASan (2-3x slower) or UBSan on, so it is best to run a non-sanitized build first, minimize the testcase and then run it for a bit of time with sanitizers. + +Building with clang 13 or later is recommended, since libfuzzer contains and by default utilizes the entropic power schedule, which is considered more efficient +than the standard one present in previous versions. +You can downloadload the latest clang+llvm release here: https://github.com/llvm/llvm-project/releases + +To set up cmake with another version of clang and fuzzing on: + +```bash +cmake -DCMAKE_BUILD_TYPE=RelWithDebInfo -DCMAKE_C_COMPILER= -DCMAKE_CXX_COMPILER= -DFUZZING=ON .. +``` + +## Currently supported +Currently we have fuzzers for bigfield, bit_array, bool, byte_array, field, safe_uint and uint. Each of them is available in 3 versions: StandardPlonk, TurboPlonk, ALL (differential fuzzing of 2 versions). +To compile all fuzzers just type `make`. +## Running the fuzzer +TODO: add information about saved testcases + +You can simply run a fuzzer by executing the built executable, for example, +```bash +./bin/stdlib_primitives_bigfield_turbo_fuzzer +``` +This is useful if you added a feature/instruction or changed some logic and want to quickly test if there are any really bad bugs. +To run the fuzzer seriously, I'd recommend: +```bash +mkdir ../../../_testcases; +mkdir crashes; +./bin/ -timeout=1 -len_control=500 -workers=8 -jobs=8 -entropic=1 -shrink=1 -artifact_prefix=crashes/ -use_value_profile=1 ../../../_testcases +``` +You can watch the progress of the fuzzer in one of the generated logs fuzz-.log +The purpose of each parameter: ++ -timeout=1 - If a testcase takes more than 1 second to execute, it will be treated as a crash ++ -len_control=500 - Slows down the increase of testcase size. Especially important for heavy classes like bigfield, keeps the number of executions per second at a decent rate ++ -worker=8 - The number of threads that can simultaneously execute testcases. Should be less or equal to the number of jobs ++ -jobs=8 - After how many crashes the fuzzer will stop fuzzing. If a crash is executed and the number of jobs is more than workers then the fuzzer will proceed to give the worker a new job. The 8/8 worker/job configuration ensures that the fuzzer will quit after 8 crashes and until the first crash all the workers are busy. ++ -entropic=1 - Entropic should be enabled by default, but in case it isn't, enable it. A better power schedule than the old one. ++ -shrink=1 - If a new testcase is encountered that has the same coverage as some previous one in the corpus and the testcase is smaller, replace the one in the corpus with the new one. Helps keep exec/s higher. ++ -artifact_prefix=crashes/ - Where to save crashes/timeouts/ooms. ++ -use_value_profile=1 - Leverage libfuzzer internal CMP analysis. Very useful, but blows the corpus up. ++ (../../../_testcases) - The path to the folder, where corpus testcases are going to be saved and loaded from (also loads testcases from there at the start of fuzzing). + +Log structure is described here https://llvm.org/docs/LibFuzzer.html + +If you've found an issue, stopped the fuzzer, you can minimize the corpus to get rid of repetitions and then start from a minimized corpus + +```bash +mkdir ../../../_testcases_minimized; +./bin/ -merge=1 -use_value_profile=1 ../../../_testcases_minimized ../../../_testcases; +rm ../../../_testcases/*; +cp ../../../_testcases_minimized/* ../../../_testcases; +``` + +If you've found a crash, you can minimize the crash to make the root cause more obvious: +```bash +mkdir minimized_crashes +./bin/ -minimize_crash=1 -artifact_prefix=minimized_crashes +``` +Also, both bigfield and safeuint fuzzer containt the SHOW_INFORMATION preprocessor cases, which enable the printing of instructions and values to make debugging the crash easier. + +# Coverage reports + +Build with coverage instrumentation: + +```cpp +mkdir build-coverage/ +cd build-coverage/ +cmake -DFUZZING=ON -DCMAKE_CXX_FLAGS="-fprofile-instr-generate -fcoverage-mapping" .. +make -j$(nproc) +``` + +Then run the fuzzer on the corpus and generate the HTML coverage reports: + +``` +LLVM_PROFILE_FILE="coverage.profraw" ./bin/ corpus/ -runs=1 +llvm-profdata merge -sparse coverage.profraw -o coverage.profdata +llvm-cov show -output-dir=out/report -format=html ./bin/ -instr-profile=coverage.profdata +``` + +View the coverage reports with your web browser: + +``` +python3 -m http.server --directory out/ +``` diff --git a/src/aztec/common/fuzzer.hpp b/src/aztec/common/fuzzer.hpp new file mode 100644 index 00000000000..9be101f4008 --- /dev/null +++ b/src/aztec/common/fuzzer.hpp @@ -0,0 +1,598 @@ +#pragma once +#include +#include +#include + +#define PARENS () + +// Rescan macro tokens 256 times +#define EXPAND(arg) EXPAND1(EXPAND1(EXPAND1(EXPAND1(arg)))) +#define EXPAND1(arg) EXPAND2(EXPAND2(EXPAND2(EXPAND2(arg)))) +#define EXPAND2(arg) EXPAND3(EXPAND3(EXPAND3(EXPAND3(arg)))) +#define EXPAND3(arg) EXPAND4(EXPAND4(EXPAND4(EXPAND4(arg)))) +#define EXPAND4(arg) arg + +#define FOR_EACH(macro, ...) __VA_OPT__(EXPAND(FOR_EACH_HELPER(macro, __VA_ARGS__))) +#define FOR_EACH_HELPER(macro, a1, ...) macro(a1) __VA_OPT__(FOR_EACH_AGAIN PARENS(macro, __VA_ARGS__)) +#define FOR_EACH_AGAIN() FOR_EACH_HELPER + +#define ALL_POSSIBLE_OPCODES \ + CONSTANT, WITNESS, CONSTANT_WITNESS, ADD, SUBTRACT, MULTIPLY, DIVIDE, ADD_TWO, MADD, MULT_MADD, MSUB_DIV, SQR, \ + ASSERT_EQUAL, ASSERT_NOT_EQUAL, SQR_ADD, ASSERT_EQUAL, ASSERT_NOT_EQUAL, SQR_ADD, SUBTRACT_WITH_CONSTRAINT, \ + DIVIDE_WITH_CONSTRAINTS, SLICE, ASSERT_ZERO, ASSERT_NOT_ZERO, COND_NEGATE, ADD_MULTI, ASSERT_VALID, \ + COND_SELECT, DOUBLE, RANDOMSEED, SELECT_IF_ZERO, SELECT_IF_EQ, REVERSE, GET_BIT, SET_BIT, SET, INVERT, AND, \ + OR, XOR, MODULO, SHL, SHR, ROL, ROR, NOT + +struct HavocSettings { + size_t GEN_LLVM_POST_MUTATION_PROB; // Controls frequency of additional mutation after structural ones + size_t GEN_MUTATION_COUNT_LOG; // This is the logarithm of the number of micromutations applied during mutation of a + // testcase + size_t GEN_STRUCTURAL_MUTATION_PROBABILITY; // The probability of applying a structural mutation + // (DELETION/DUPLICATION/INSERTION/SWAP) + size_t GEN_VALUE_MUTATION_PROBABILITY; // The probability of applying a value mutation + size_t ST_MUT_DELETION_PROBABILITY; // The probability of applying DELETION mutation + size_t ST_MUT_DUPLICATION_PROBABILITY; // The probability of applying DUPLICATION mutation + size_t ST_MUT_INSERTION_PROBABILITY; // The probability of applying INSERTION mutation + size_t ST_MUT_MAXIMUM_DELETION_LOG; // The logarithm of the maximum of deletions + size_t ST_MUT_MAXIMUM_DUPLICATION_LOG; // The logarithm of the maximum of duplication + size_t ST_MUT_SWAP_PROBABILITY; // The probability of a SWAP mutation + size_t VAL_MUT_LLVM_MUTATE_PROBABILITY; // The probablity of using the LLVM mutator on field element value + size_t VAL_MUT_MONTGOMERY_PROBABILITY; // The probability of converting to montgomery form before applying value + // mutations + size_t VAL_MUT_NON_MONTGOMERY_PROBABILITY; // The probability of not converting to montgomery form before applying + // value mutations + size_t VAL_MUT_SMALL_ADDITION_PROBABILITY; // The probability of performing small additions + size_t VAL_MUT_SMALL_MULTIPLICATION_PROBABILITY; // The probability of performing small multiplications + size_t VAL_MUT_SPECIAL_VALUE_PROBABILITY; // The probability of assigning special values (0,1, p-1, p-2, p-1/2) + std::vector structural_mutation_distribution; // Holds the values to quickly select a structural mutation + // based on chosen probabilities + std::vector value_mutation_distribution; // Holds the values to quickly select a value mutation based on + // chosen probabilities +}; +#ifdef HAVOC_TESTING + +HavocSettings fuzzer_havoc_settings; +#endif +// This is an external function in Libfuzzer used internally by custom mutators +extern "C" size_t LLVMFuzzerMutate(uint8_t* Data, size_t Size, size_t MaxSize); + +/** + * @brief Class for quickly deterministically creating new random values. We don't care about distribution much here. + * + */ +class FastRandom { + uint32_t state; + + public: + FastRandom(uint32_t seed) { reseed(seed); } + uint32_t next() + { + state = static_cast((uint64_t(state) * uint64_t(363364578) + uint64_t(537)) % uint64_t(3758096939)); + return state; + } + void reseed(uint32_t seed) + { + if (seed == 0) { + seed = 1; + } + state = seed; + } +}; + +/** + * @brief Concept for a simple PRNG which returns a uint32_t when next is called + * + * @tparam T + */ +template concept SimpleRng = requires(T a) +{ + { + a.next() + } + ->std::convertible_to; +}; +/** + * @brief Concept for forcing ArgumentSizes to be size_t + * + * @tparam T + */ +template concept InstructionArgumentSizes = requires +{ + { + std::make_tuple(T::CONSTANT, + T::WITNESS, + T::CONSTANT_WITNESS, + T::ADD, + T::SUBTRACT, + T::MULTIPLY, + T::DIVIDE, + T::ADD_TWO, + T::MADD, + T::MULT_MADD, + T::MSUB_DIV, + T::SQR, + T::SQR_ADD, + T::SUBTRACT_WITH_CONSTRAINT, + T::DIVIDE_WITH_CONSTRAINTS, + T::SLICE, + T::ASSERT_ZERO, + T::ASSERT_NOT_ZERO) + } + ->std::same_as>; +}; + +/** + * @brief Concept for Havoc Configurations + * + * @tparam T + */ +template concept HavocConfigConstraint = requires +{ + { + std::make_tuple(T::GEN_MUTATION_COUNT_LOG, T::GEN_STRUCTURAL_MUTATION_PROBABILITY) + } + ->std::same_as>; + T::GEN_MUTATION_COUNT_LOG <= 7; +}; +/** + * @brief Concept specifying the class used by the fuzzer + * + * @tparam T + */ +template concept ArithmeticFuzzHelperConstraint = requires +{ + typename T::ArgSizes; + typename T::Instruction; + typename T::ExecutionState; + typename T::ExecutionHandler; + InstructionArgumentSizes; + // HavocConfigConstraint; +}; + +/** + * @brief Fuzzer uses only composers with check_circuit function + * + * @tparam T + */ +template concept CheckableComposer = requires(T a) +{ + { + a.check_circuit() + } + ->std::same_as; +}; + +/** + * @brief The fuzzer can use a postprocessing function that is specific to the type being fuzzed + * + * @tparam T Type being tested + * @tparam Composer + * @tparam Context The class containing the full context + */ +template +concept PostProcessingEnabled = requires(Composer composer, Context context) +{ + { + T::postProcess(&composer, context) + } + ->std::same_as; +}; + +/** + * @brief This concept is used when we want to limit the number of executions of certain instructions (for example, + * divisions and multiplications in bigfield start to bog down the fuzzer) + * + * @tparam T + */ +template concept InstructionWeightsEnabled = requires +{ + typename T::InstructionWeights; + T::InstructionWeights::_LIMIT; +}; +/** + * @brief A templated class containing most of the fuzzing logic for a generic Arithmetic class + * + * @tparam T + */ +template requires ArithmeticFuzzHelperConstraint class ArithmeticFuzzHelper { + private: + /** + * @brief Mutator swapping two instructions together + * + * @param instructions + * @param rng + */ + inline static void swapTwoInstructions(std::vector& instructions, FastRandom& rng) + { + const size_t instructions_count = instructions.size(); + if (instructions_count <= 2) { + return; + } + const size_t first_element_index = rng.next() % instructions_count; + size_t second_element_index = rng.next() % instructions_count; + if (first_element_index == second_element_index) { + second_element_index = (second_element_index + 1) % instructions_count; + } + std::iter_swap(instructions.begin() + static_cast(first_element_index), + instructions.begin() + static_cast(second_element_index)); + } + + /** + * @brief Mutator, deleting a sequence of instructions + * + * @param instructions + * @param rng + * @param havoc_settings + */ + inline static void deleteInstructions(std::vector& instructions, + FastRandom& rng, + HavocSettings& havoc_settings) + { + + const size_t instructions_count = instructions.size(); + if (instructions_count == 0) { + return; + } + if (rng.next() & 1) { + instructions.erase(instructions.begin() + (rng.next() % instructions_count)); + } else { + // We get the logarithm of number of instructions and subtract 1 to delete at most half + const size_t max_deletion_log = + std::min(static_cast(64 - __builtin_clzll(static_cast(instructions_count)) - 1), + havoc_settings.ST_MUT_MAXIMUM_DELETION_LOG); + + if (max_deletion_log == 0) { + return; + } + const size_t deletion_size = 1 << (rng.next() % max_deletion_log); + const size_t start = rng.next() % (instructions_count + 1 - deletion_size); + instructions.erase(instructions.begin() + static_cast(start), + instructions.begin() + static_cast(start + deletion_size)); + } + } + /** + * @brief Mutator duplicating an instruction + * + * @param instructions + * @param rng + * @param havoc_settings + */ + inline static void duplicateInstruction(std::vector& instructions, + FastRandom& rng, + HavocSettings& havoc_settings) + { + const size_t instructions_count = instructions.size(); + if (instructions_count == 0) { + return; + } + const size_t duplication_size = 1 << (rng.next() % havoc_settings.ST_MUT_MAXIMUM_DUPLICATION_LOG); + typename T::Instruction chosen_instruction = instructions[rng.next() % instructions_count]; + instructions.insert( + instructions.begin() + (rng.next() % (instructions_count + 1)), duplication_size, chosen_instruction); + } + inline static void insertRandomInstruction(std::vector& instructions, + FastRandom& rng, + HavocSettings& havoc_settings) + { + (void)havoc_settings; + instructions.insert(instructions.begin() + static_cast(rng.next() % (instructions.size() + 1)), + T::Instruction::template generateRandom(rng)); + } + /** + * @brief Mutator for instruction structure + * + * @param instructions + * @param rng + * @param havoc_settings + */ + inline static void mutateInstructionStructure(std::vector& instructions, + FastRandom& rng, + HavocSettings& havoc_settings) + { + const size_t structural_mutators_count = havoc_settings.structural_mutation_distribution.size(); + const size_t prob_pool = havoc_settings.structural_mutation_distribution[structural_mutators_count - 1]; + const size_t choice = rng.next() % prob_pool; + if (choice < havoc_settings.structural_mutation_distribution[0]) { + deleteInstructions(instructions, rng, havoc_settings); + } else if (choice < havoc_settings.structural_mutation_distribution[1]) { + + duplicateInstruction(instructions, rng, havoc_settings); + } else if (choice < havoc_settings.structural_mutation_distribution[2]) { + insertRandomInstruction(instructions, rng, havoc_settings); + } else { + + swapTwoInstructions(instructions, rng); + } + } + /** + * @brief Choose a random instruction from the vector and mutate it + * + * @param instructions Vector of instructions + * @param rng Pseudorandom number generator + * @param havoc_settings Mutation settings + */ + inline static void mutateInstructionValue(std::vector& instructions, + FastRandom& rng, + HavocSettings& havoc_settings) + { + + const size_t instructions_count = instructions.size(); + if (instructions_count == 0) { + return; + } + const size_t chosen = rng.next() % instructions_count; + instructions[chosen] = + T::Instruction::template mutateInstruction(instructions[chosen], rng, havoc_settings); + } + + static void mutateInstructionVector(std::vector& instructions, FastRandom& rng) + { +#ifdef HAVOC_TESTING + // If we are testing which havoc settings are best, then we use global parameters + const size_t mutation_count = 1 << fuzzer_havoc_settings.GEN_MUTATION_COUNT_LOG; +#else + const size_t mutation_count = 1 << T::HavocConfig::MUTATION_COUNT_LOG; + HavocSettings fuzzer_havoc_settings; + // FILL the values +#endif + for (size_t i = 0; i < mutation_count; i++) { + uint32_t val = rng.next(); + if ((val % (fuzzer_havoc_settings.GEN_STRUCTURAL_MUTATION_PROBABILITY + + fuzzer_havoc_settings.GEN_VALUE_MUTATION_PROBABILITY)) < + fuzzer_havoc_settings.GEN_STRUCTURAL_MUTATION_PROBABILITY) { + // mutate structure + mutateInstructionStructure(instructions, rng, fuzzer_havoc_settings); + } else { + // mutate a single instruction vector + + mutateInstructionValue(instructions, rng, fuzzer_havoc_settings); + } + } + } + + public: + /** + * @brief Splice two instruction vectors into one randomly + * + * @param vecA First instruction vector + * @param vecB Second instruction vector + * @param rng PRNG + * @return Resulting vector of instructions + */ + static std::vector crossoverInstructionVector( + const std::vector& vecA, + const std::vector& vecB, + FastRandom& rng) + { + // Get vector sizes + const size_t vecA_size = vecA.size(); + const size_t vecB_size = vecB.size(); + // If one of them is empty, just return the other one + if (vecA_size == 0) { + return vecB; + } + if (vecB_size == 0) { + return vecA; + } + std::vector result; + // Choose the size of th resulting vector + const size_t final_result_size = rng.next() % (vecA_size + vecB_size) + 1; + size_t indexA = 0, indexB = 0; + size_t* inIndex = &indexA; + size_t inSize = vecA_size; + auto inIterator = vecA.begin(); + size_t current_result_size = 0; + bool currentlyUsingA = true; + // What we do is basically pick a sequence from one, follow with a sequence from the other + while (current_result_size < final_result_size && (indexA < vecA_size || indexB < vecB_size)) { + // Get the size left + size_t result_size_left = final_result_size - current_result_size; + // If we can still read from this vector + if (*inIndex < inSize) { + // Get the size left in this vector and in the output vector and pick the lowest + size_t inSizeLeft = inSize - *inIndex; + size_t maxExtraSize = std::min(result_size_left, inSizeLeft); + if (maxExtraSize != 0) { + // If not zero, get a random number of elements from input + size_t copySize = (rng.next() % maxExtraSize) + 1; + result.insert(result.begin() + static_cast(current_result_size), + inIterator + static_cast((*inIndex)), + + inIterator + static_cast((*inIndex) + copySize)); + // Update indexes and sizes + *inIndex += copySize; + current_result_size += copySize; + } + } + // Switch input vector + inIndex = currentlyUsingA ? &indexB : &indexA; + inSize = currentlyUsingA ? vecB_size : vecA_size; + inIterator = currentlyUsingA ? vecB.begin() : vecA.begin(); + currentlyUsingA = !currentlyUsingA; + } + // Return spliced vector + return result; + } + /** + * @brief Parses a given data buffer into a vector of instructions for testing the arithmetic + * + * @param Data Pointer to the data buffer + * @param Size Data buffer size + * @return A vector of instructions + */ + static std::vector parseDataIntoInstructions(const uint8_t* Data, size_t Size) + { + std::vector fuzzingInstructions; + uint8_t* pData = (uint8_t*)Data; + size_t size_left = Size; + while (size_left != 0) { + uint8_t chosen_operation = *pData; + size_left -= 1; + pData++; + // If the opcode is enabled (exists and arguments' size is not -1), check if it's the right opcode. If it + // is, parse it with a designated function +#define PARSE_OPCODE(name) \ + if constexpr (requires { T::ArgSizes::name; }) \ + if constexpr (T::ArgSizes::name != size_t(-1)) { \ + if (chosen_operation == T::Instruction::OPCODE::name) { \ + if (size_left < T::ArgSizes::name) { \ + return fuzzingInstructions; \ + } \ + fuzzingInstructions.push_back( \ + T::Parser::template parseInstructionArgs(pData)); \ + size_left -= T::ArgSizes::name; \ + pData += T::ArgSizes::name; \ + continue; \ + } \ + } + // Create handlers for all opcodes that are in ArgsSizes +#define PARSE_ALL_OPCODES(...) FOR_EACH(PARSE_OPCODE, __VA_ARGS__) + + PARSE_ALL_OPCODES(ALL_POSSIBLE_OPCODES) + } + return fuzzingInstructions; + } + /** + * @brief Write instructions into the buffer until there are no instructions left or there is no more space + * + * @param instructions Vector of fuzzing instructions + * @param Data Pointer to data buffer + * @param MaxSize Size of buffer + * @return How much of the buffer was filled with instructions + */ + static size_t writeInstructionsToBuffer(std::vector& instructions, + uint8_t* Data, + size_t MaxSize) + { + uint8_t* pData = Data; + size_t size_left = MaxSize; + for (auto& instruction : instructions) { + // If the opcode is enabled and it's this opcode, use a designated function to serialize it +#define WRITE_OPCODE_IF(name) \ + if constexpr (requires { T::ArgSizes::name; }) \ + if constexpr (T::ArgSizes::name != (size_t)-1) { \ + if (instruction.id == T::Instruction::OPCODE::name) { \ + if (size_left >= (T::ArgSizes::name + 1)) { \ + T::Parser::template writeInstruction(instruction, pData); \ + size_left -= (T::ArgSizes::name + 1); \ + pData += (T::ArgSizes::name + 1); \ + } else { \ + return MaxSize - size_left; \ + } \ + continue; \ + } \ + } + // Create handlers for all opcodes that are in ArgsSizes +#define WRITE_ALL_OPCODES(...) FOR_EACH(WRITE_OPCODE_IF, __VA_ARGS__) + + WRITE_ALL_OPCODES(ALL_POSSIBLE_OPCODES) + } + return MaxSize - size_left; + } + + /** + * @brief Execute instructions in a loop + * + * @tparam Composer composer used + * @param instructions + */ + template + inline static void executeInstructions( + std::vector& instructions) requires CheckableComposer + { + typename T::ExecutionState state; + Composer composer = Composer(); + circuit_should_fail = false; + size_t total_instruction_weight = 0; + (void)total_instruction_weight; + for (auto& instruction : instructions) { + // If instruction enabled and this is it, delegate to the handler +#define EXECUTE_OPCODE_IF(name) \ + if constexpr (requires { T::ArgSizes::name; }) \ + if constexpr (T::ArgSizes::name != size_t(-1)) { \ + if (instruction.id == T::Instruction::OPCODE::name) { \ + if constexpr (InstructionWeightsEnabled) { \ + if (!((total_instruction_weight + T::InstructionWeights::name) > T::InstructionWeights::_LIMIT)) { \ + total_instruction_weight += T::InstructionWeights::name; \ + if (T::ExecutionHandler::execute_##name(&composer, state, instruction)) { \ + return; \ + } \ + } else { \ + return; \ + } \ + } else { \ + \ + if (T::ExecutionHandler::execute_##name(&composer, state, instruction)) { \ + return; \ + } \ + } \ + } \ + } +#define EXECUTE_ALL_OPCODES(...) FOR_EACH(EXECUTE_OPCODE_IF, __VA_ARGS__) + + EXECUTE_ALL_OPCODES(ALL_POSSIBLE_OPCODES) + } + bool final_value_check = true; + // If there is a posprocessing function, use it + if constexpr (PostProcessingEnabled>) { + final_value_check = T::postProcess(&composer, state); +#ifdef SHOW_INFORMATION + if (!final_value_check) { + std::cerr << "Final value check failed" << std::endl; + } +#endif + } + bool check_result = composer.check_circuit() && final_value_check; + // If the circuit is correct, but it should fail, abort + if (check_result && circuit_should_fail) { + abort(); + } + // If the circuit is incorrect, but there's no reason, abort + if ((!check_result) && (!circuit_should_fail)) { + if (!final_value_check) { + std::cerr << "Final value check failed" << std::endl; + } else { + std::cerr << "Circuit failed" << std::endl; + } + + abort(); + } + } + + /** + * @brief Interpret the data buffer as a series of arithmetic instructions and mutate it accordingly + * + * @param Data Pointer to the buffer + * @param Size The initial filled size + * @param MaxSize The size of the buffer + * @return size_t The new length of data in the buffer + */ + static size_t MutateInstructionBuffer(uint8_t* Data, size_t Size, size_t MaxSize, FastRandom& rng) + { + // Parse the vector + std::vector instructions = parseDataIntoInstructions(Data, Size); + // Mutate the vector of instructions + mutateInstructionVector(instructions, rng); + // Serialize the vector of instructions back to buffer + return writeInstructionsToBuffer(instructions, Data, MaxSize); + } +}; + +template