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 <[email protected]> Co-authored-by: LHerskind <[email protected]> Co-authored-by: David Banks <[email protected]> Co-authored-by: arijitdutta67 <[email protected]> Co-authored-by: Innokentii Sennovskii <[email protected]> Co-authored-by: Michael Connor <[email protected]> Co-authored-by: Suyash Bagad <[email protected]>
AztecProtocol · Nov 28, 2022 · 37adf5c · 37adf5c
1 parent 000c71a
commit 37adf5c
Show file tree

Hide file tree

Showing 12 changed files with 87 additions and 32 deletions.
diff --git 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
@@ -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<field_ct>{ account_public_key.x, account_public_key.y },
+    return pedersen::compress(std::vector<field_ct>{ 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
@@ -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
@@ -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
@@ -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
@@ -13,12 +13,12 @@ using namespace barretenberg;
 
 inline fr compute_account_public_key_nullifier(grumpkin::g1::affine_element const& public_key)
 {
-    return crypto::pedersen::compress_native(std::vector<fr>{ public_key.x, public_key.y },
+    return crypto::pedersen::compress_native(std::vector<fr>{ public_key.x },
                                              notes::GeneratorIndex::ACCOUNT_PUBLIC_KEY_NULLIFIER);
 }
 
 } // namespace account
 } // namespace native
 } // namespace notes
 } // namespace proofs
-} // namespace rollup
+} // 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
@@ -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<uint256_t>(vk_hash_rollup) == circuit_vk_hash::ROLLUP)
+        EXPECT_EQ(from_buffer<uint256_t>(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
@@ -146,6 +146,9 @@ point<C> variable_base_mul(const point<C>& pub_key, const field_t<C>& low_bits,
 template <typename C>
 point<C> variable_base_mul(const point<C>& pub_key, const point<C>& current_accumulator, const wnaf_record<C>& 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<C> variable_base_mul(const point<C>& pub_key, const point<C>& current_accu
     if (init) {
         field_t<C> 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<C> 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
@@ -254,6 +254,28 @@ template <typename Composer, typename T> 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<Composer>((multiple_of_modulus % uint512_t(field_t<Composer>::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
@@ -47,6 +47,23 @@ template <typename Composer> 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<waffle::StandardComposer,
     ComposerTypes;
 // Define the suite of tests.
 TYPED_TEST_SUITE(stdlib_bigfield, ComposerTypes);
+TYPED_TEST(stdlib_bigfield, division_formula_bug)
+{
+    TestFixture::test_division_formula_bug();
+}
 TYPED_TEST(stdlib_bigfield, badmul)
 {
     TestFixture::test_bad_mul();

diff --git a/src/aztec/stdlib/primitives/bigfield/bigfield_impl.hpp b/src/aztec/stdlib/primitives/bigfield/bigfield_impl.hpp
@@ -536,7 +536,8 @@ bigfield<C, T> bigfield<C, T>::internal_div(const std::vector<bigfield>& 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<C, T> bigfield<C, T>::internal_div(const std::vector<bigfield>& numerat
         auto [reduction_required, num_quotient_bits] =
             get_quotient_reduction_info({ static_cast<uint512_t>(DEFAULT_MAXIMUM_REMAINDER) },
                                         { denominator.get_maximum_value() },
-                                        {},
+                                        { unreduced_zero() },
                                         numerator_max);
         if (reduction_required) {
 
@@ -575,7 +576,7 @@ bigfield<C, T> bigfield<C, T>::internal_div(const std::vector<bigfield>& 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
@@ -24,6 +24,14 @@ template <typename Composer> 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<Composer> create_point_witness(Composer& composer, E const& p, const bool
     // validate point is on the grumpkin curve
     field_t<Composer> x(witness_t<Composer>(&composer, p.x));
     field_t<Composer> y(witness_t<Composer>(&composer, p.y));
+    point<Composer> 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 };
 }