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] 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 }; }