Revert "Verification Key changes (#1695)"

This reverts commit 37adf5c.

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 ACCOUNT = 23958;
-constexpr uint32_t JOIN_SPLIT = 64000;
+constexpr uint32_t JOIN_SPLIT = 63984;
+constexpr uint32_t ACCOUNT = 24123;
 constexpr uint32_t CLAIM = 22684;
-constexpr uint32_t ROLLUP = 1173221;
-constexpr uint32_t ROOT_ROLLUP = 5481327;
-constexpr uint32_t ROOT_VERIFIER = 7435892;
+constexpr uint32_t ROLLUP = 1171925;
+constexpr uint32_t ROOT_ROLLUP = 5477579;
+constexpr uint32_t ROOT_VERIFIER = 7433260;
 }; // 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 ACCOUNT = 32768;
 constexpr uint32_t JOIN_SPLIT = 65536;
+constexpr uint32_t ACCOUNT = 32768;
 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(0x78ebf096ab73e440, 0xaa1dc7c26a125f6e, 0x488a97e465b96964, 0xf9d3e501b89bf466);
-constexpr auto JOIN_SPLIT = uint256_t(0x5e67a4a4503ebf25, 0xb3c070c061e76d1a, 0xb18c6c6a5bcad5fb, 0xe0d5f46cafb33ecf);
+constexpr auto ACCOUNT = uint256_t(0x5d97584af28e524a, 0xd2007df20f73d2bf, 0xe134d5b853eff05a, 0xdd2c3d1ff8bce519);
+constexpr auto JOIN_SPLIT = uint256_t(0x7d54acfcca0eaf34, 0xa69c7b4e1c2fb5c5, 0x77161b356ca1c7a2, 0xb88194f268d2caa9);
 constexpr auto CLAIM = uint256_t(0x878301ebba40ab60, 0x931466762c62d661, 0x40aad71ec3496905, 0x9f47aaa109759d0a);
-constexpr auto ROLLUP = uint256_t(0x160731cc44173fdc, 0x6a6d55e46bf198bd, 0x9ce1d4608ae26fb0, 0x865ced5c16cb6152);
-constexpr auto ROOT_ROLLUP = uint256_t(0xd77e82eae9e6efc7, 0x2b5ddf767012a4cf, 0x8b5982bb3d64616f, 0x20b515f5a9c78048);
+constexpr auto ROLLUP = uint256_t(0x3c9e491095547852, 0xbf65ec889ec96a1a, 0xb16e824aa0bb319f, 0x28d7b587edf1eb4d);
+constexpr auto ROOT_ROLLUP = uint256_t(0xa5e06b55f0e30cbe, 0x5fbf39af52fe67c8, 0xd8a0ecd1bb3a6f40, 0xdf67f7fcbb55dc1f);
 constexpr auto ROOT_VERIFIER =
-    uint256_t(0x8e8313d6015ca626, 0x62ccf70b81c4e099, 0x33bee0072a20f36a, 0x44bd24daa009cd59);
+    uint256_t(0x341a876aae2df472, 0x87e0704f1ae50773, 0x5d6c740f61a0dbdd, 0x1f1a94b50cdcf5ae);
 }; // namespace circuit_vk_hash
 
 namespace ProofIds {

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 },
+    return pedersen::compress(std::vector<field_ct>{ account_public_key.x, account_public_key.y },
                               notes::GeneratorIndex::ACCOUNT_PUBLIC_KEY_NULLIFIER);
 }
 void account_circuit(Composer& composer, account_tx const& tx)

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 },
+        return crypto::pedersen::compress_native({ account_public_key.x, account_public_key.y },
                                                  notes::GeneratorIndex::ACCOUNT_PUBLIC_KEY_NULLIFIER);
     }
 

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 },
+        return compress_native({ new_account_public_key.x, new_account_public_key.y },
                                rollup::proofs::notes::GeneratorIndex::ACCOUNT_PUBLIC_KEY_NULLIFIER);
     }
     return 0;

src/aztec/rollup/proofs/join_split/join_split_circuit.cpp

@@ -287,7 +287,8 @@ 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 = stdlib::create_point_witness(composer, tx.signing_pub_key),
+        .signing_pub_key = { .x = witness_ct(&composer, tx.signing_pub_key.x),
+                             .y = witness_ct(&composer, tx.signing_pub_key.y) },
         .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]),
@@ -325,7 +326,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

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 },
+    return crypto::pedersen::compress_native(std::vector<fr>{ public_key.x, public_key.y },
                                              notes::GeneratorIndex::ACCOUNT_PUBLIC_KEY_NULLIFIER);
 }
 
 } // namespace account
 } // namespace native
 } // namespace notes
 } // namespace proofs
-} // namespace rollup
+} // namespace rollup

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_EQ(number_of_gates_rollup, circuit_gate_count::ROLLUP)
+        EXPECT_TRUE(number_of_gates_rollup == circuit_gate_count::ROLLUP)
             << "The gate count for the rollup circuit is changed.";
-        EXPECT_EQ(from_buffer<uint256_t>(vk_hash_rollup), circuit_vk_hash::ROLLUP)
+        EXPECT_TRUE(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_LE(number_of_gates_rollup,
-                  circuit_gate_next_power_of_two::ROLLUP - waffle::ComposerBase::NUM_RESERVED_GATES)
+        EXPECT_TRUE(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_LE(number_of_gates_rollup,
-                  circuit_gate_next_power_of_two::ROLLUP - waffle::ComposerBase::NUM_RESERVED_GATES)
+        EXPECT_TRUE(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.";
     }
 }

src/aztec/stdlib/encryption/schnorr/schnorr.cpp

@@ -146,9 +146,6 @@ 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.
@@ -171,9 +168,6 @@ 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 };

src/aztec/stdlib/primitives/bigfield/bigfield.hpp

@@ -254,28 +254,6 @@ 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.
      **/

src/aztec/stdlib/primitives/bigfield/bigfield.test.cpp

@@ -47,23 +47,6 @@ 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()
     {
 
@@ -827,10 +810,6 @@ 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();

src/aztec/stdlib/primitives/bigfield/bigfield_impl.hpp

@@ -536,8 +536,7 @@ 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 + unreduced_zero().get_value() - left) / modulus;
+    const uint1024_t quotient_1024 = (uint1024_t(inverse_value) * right - left) / modulus;
     const uint512_t quotient_value = quotient_1024.lo;
 
     bigfield inverse;
@@ -556,7 +555,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) {
 
@@ -576,7 +575,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, { unreduced_zero() }, quotient, numerators);
+    unsafe_evaluate_multiply_add(denominator, inverse, {}, quotient, numerators);
     return inverse;
 }
 

src/aztec/stdlib/primitives/point/point.hpp

@@ -24,14 +24,6 @@ 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;
@@ -54,11 +46,13 @@ 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) {
-        result.on_curve("create_point_witness: point not 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");
     }
     return { x, y };
 }