From b96336169e79387267b0cd272e712d820cc70e27 Mon Sep 17 00:00:00 2001
From: codygunton <codygunton@gmail.com>
Date: Wed, 9 Aug 2023 02:09:02 +0000
Subject: [PATCH] Speed up ECDSA simulation

Benchmark                                              Time             CPU      Time Old      Time New       CPU Old       CPU New
-----------------------------------------------------------------------------------------------------------------------------------
pedersen_compress_pair/10/repeats:1                 +0.2185         +0.2183         78085         95143         78097         95150
pedersen_compress_array/10/repeats:1                +0.0161         +0.0161        429360        436272        429403        436334
blake3s/10/repeats:1                              +211.5298       +210.6946           499        106157           502        106166
ecdsa/10/repeats:1                                  +0.0156         +0.0155        238213        241933        238196        241895
biggroup_batch_mul/10/repeats:1                     +0.1987         +0.1987        963210       1154583        963031       1154370
OVERALL_GEOMEAN                                     +2.1704         +2.1679             0             0             0             0
%
---
 .../src/barretenberg/crypto/ecdsa/ecdsa.hpp   |   2 +-
 .../barretenberg/crypto/ecdsa/ecdsa_impl.hpp  |   2 +-
 .../stdlib/encryption/ecdsa/ecdsa_impl.hpp    | 184 ++++++++++--------
 3 files changed, 102 insertions(+), 86 deletions(-)

diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa.hpp
index 16106a5868a..8264ed985f2 100644
--- a/circuits/cpp/barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa.hpp
+++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa.hpp
@@ -32,7 +32,7 @@ template <typename Hash, typename Fq, typename Fr, typename G1>
 typename G1::affine_element recover_public_key(const std::string& message, const signature& sig);
 
 template <typename Hash, typename Fq, typename Fr, typename G1>
-bool verify_signature(const std::string& message,
+bool verify_signature(const auto& message,
                       const typename G1::affine_element& public_key,
                       const signature& signature);
 
diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa_impl.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa_impl.hpp
index 085ca712835..4cbcb8c44ea 100644
--- a/circuits/cpp/barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa_impl.hpp
+++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/crypto/ecdsa/ecdsa_impl.hpp
@@ -125,7 +125,7 @@ typename G1::affine_element recover_public_key(const std::string& message, const
 }
 
 template <typename Hash, typename Fq, typename Fr, typename G1>
-bool verify_signature(const std::string& message, const typename G1::affine_element& public_key, const signature& sig)
+bool verify_signature(const auto& message, const typename G1::affine_element& public_key, const signature& sig)
 {
     using serialize::read;
     uint256_t r_uint;
diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/encryption/ecdsa/ecdsa_impl.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/encryption/ecdsa/ecdsa_impl.hpp
index 7e709ff12dd..db42ec12872 100644
--- a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/encryption/ecdsa/ecdsa_impl.hpp
+++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/encryption/ecdsa/ecdsa_impl.hpp
@@ -27,95 +27,111 @@ bool_t<Composer> verify_signature(const stdlib::byte_array<Composer>& message,
 {
     Composer* ctx = message.get_context() ? message.get_context() : public_key.x.context;
 
-    /**
-     * Check if recovery id v is either 27 ot 28.
-     *
-     * The v in an (r, s, v) ecdsa signature is the 8-bit recovery id s.t. v ∈ {0, 1, 2, 3}.
-     * It is used to recover signing public key from an ecdsa signature. In practice, the value
-     * of v is offset by 27 following the convention from the original bitcoin whitepaper.
-     *
-     * The value of v depends on the the point R = (x, y) s.t. r = x % |Fr|
-     * 0: y is even  &&  x < |Fr| (x = r)
-     * 1: y is odd   &&  x < |Fr| (x = r)
-     * 2: y is even  &&  |Fr| <= x < |Fq| (x = r + |Fr|)
-     * 3: y is odd   &&  |Fr| <= x < |Fq| (x = r + |Fr|)
-     *
-     * It is highly unlikely for x be be in [|Fr|, |Fq|) for the secp256k1 curve because:
-     * P(|Fr| <= x < |Fq|) = 1 - |Fr|/|Fq| ≈ 0.
-     * Therefore, it is reasonable to assume that the value of v will always be 0 or 1
-     * (i.e. 27 or 28 with offset). In fact, the ethereum yellow paper [1] only allows v to be 27 or 28
-     * and considers signatures with v ∈ {29, 30} to be non-standard.
-     *
-     * TODO(Suyash): EIP-155 allows v > 35 to ensure different v on different chains.
-     * Do we need to consider that in our circuits?
-     *
-     * References:
-     * [1] Ethereum yellow paper, Appendix E: https://ethereum.github.io/yellowpaper/paper.pdf
-     * [2] EIP-155: https://eips.ethereum.org/EIPS/eip-155
-     *
-     */
-    // Note: This check is also present in the _noassert variation of this method.
-    field_t<Composer>(sig.v).assert_is_in_set({ field_t<Composer>(27), field_t<Composer>(28) },
-                                              "signature is non-standard");
+    if constexpr (IsSimulator<Composer>) {
 
-    stdlib::byte_array<Composer> hashed_message =
-        static_cast<stdlib::byte_array<Composer>>(stdlib::sha256<Composer>(message));
-
-    Fr z(hashed_message);
-    z.assert_is_in_field();
-
-    Fr r(sig.r);
-    // force r to be < secp256k1 group modulus, so we can compare with `result_mod_r` below
-    r.assert_is_in_field();
-
-    Fr s(sig.s);
+        std::vector<uint8_t> r_vector = sig.r.get_value();
+        std::vector<uint8_t> s_vector = sig.s.get_value();
+        std::array<uint8_t, 32> r_array{ 0 };
+        std::array<uint8_t, 32> s_array{ 0 };
+        std::copy(r_vector.begin(), r_vector.end(), r_array.begin());
+        std::copy(s_vector.begin(), s_vector.end(), s_array.begin());
 
-    // r and s should not be zero
-    r.assert_is_not_equal(Fr::zero());
-    s.assert_is_not_equal(Fr::zero());
-
-    // s should be less than |Fr| / 2
-    // Read more about this at: https://www.derpturkey.com/inherent-malleability-of-ecdsa-signatures/amp/
-    s.assert_less_than((Fr::modulus + 1) / 2);
-
-    Fr u1 = z / s;
-    Fr u2 = r / s;
-
-    public_key.validate_on_curve();
+        auto v = static_cast<uint8_t>(sig.v.get_value().data[0]);
 
-    G1 result;
-    // TODO(Cody): Having Plookup should not determine which curve is used.
-    // Use special plookup secp256k1 ECDSA mul if available (this relies on k1 endomorphism, and cannot be used for
-    // other curves)
-    if constexpr (HasPlookup<Composer> && Curve::type == proof_system::CurveType::SECP256K1) {
-        result = G1::secp256k1_ecdsa_mul(public_key, u1, u2);
+        bool result =
+            crypto::ecdsa::verify_signature<Sha256Hasher, typename Curve::fq, typename Curve::fr, typename Curve::g1>(
+                message.get_value(), public_key.get_value(), { r_array, s_array, v });
+        return { ctx, result };
     } else {
-        result = G1::batch_mul({ G1::one(ctx), public_key }, { u1, u2 });
+        /**
+         * Check if recovery id v is either 27 ot 28.
+         *
+         * The v in an (r, s, v) ecdsa signature is the 8-bit recovery id s.t. v ∈ {0, 1, 2, 3}.
+         * It is used to recover signing public key from an ecdsa signature. In practice, the value
+         * of v is offset by 27 following the convention from the original bitcoin whitepaper.
+         *
+         * The value of v depends on the the point R = (x, y) s.t. r = x % |Fr|
+         * 0: y is even  &&  x < |Fr| (x = r)
+         * 1: y is odd   &&  x < |Fr| (x = r)
+         * 2: y is even  &&  |Fr| <= x < |Fq| (x = r + |Fr|)
+         * 3: y is odd   &&  |Fr| <= x < |Fq| (x = r + |Fr|)
+         *
+         * It is highly unlikely for x be be in [|Fr|, |Fq|) for the secp256k1 curve because:
+         * P(|Fr| <= x < |Fq|) = 1 - |Fr|/|Fq| ≈ 0.
+         * Therefore, it is reasonable to assume that the value of v will always be 0 or 1
+         * (i.e. 27 or 28 with offset). In fact, the ethereum yellow paper [1] only allows v to be 27 or 28
+         * and considers signatures with v ∈ {29, 30} to be non-standard.
+         *
+         * TODO(Suyash): EIP-155 allows v > 35 to ensure different v on different chains.
+         * Do we need to consider that in our circuits?
+         *
+         * References:
+         * [1] Ethereum yellow paper, Appendix E: https://ethereum.github.io/yellowpaper/paper.pdf
+         * [2] EIP-155: https://eips.ethereum.org/EIPS/eip-155
+         *
+         */
+        // Note: This check is also present in the _noassert variation of this method.
+        field_t<Composer>(sig.v).assert_is_in_set({ field_t<Composer>(27), field_t<Composer>(28) },
+                                                  "signature is non-standard");
+
+        stdlib::byte_array<Composer> hashed_message =
+            static_cast<stdlib::byte_array<Composer>>(stdlib::sha256<Composer>(message));
+
+        Fr z(hashed_message);
+        z.assert_is_in_field();
+
+        Fr r(sig.r);
+        // force r to be < secp256k1 group modulus, so we can compare with `result_mod_r` below
+        r.assert_is_in_field();
+
+        Fr s(sig.s);
+
+        // r and s should not be zero
+        r.assert_is_not_equal(Fr::zero());
+        s.assert_is_not_equal(Fr::zero());
+
+        // s should be less than |Fr| / 2
+        // Read more about this at: https://www.derpturkey.com/inherent-malleability-of-ecdsa-signatures/amp/
+        s.assert_less_than((Fr::modulus + 1) / 2);
+
+        Fr u1 = z / s;
+        Fr u2 = r / s;
+
+        public_key.validate_on_curve();
+
+        G1 result;
+        // TODO(Cody): Having Plookup should not determine which curve is used.
+        // Use special plookup secp256k1 ECDSA mul if available (this relies on k1 endomorphism, and cannot be used for
+        // other curves)
+        if constexpr (HasPlookup<Composer> && Curve::type == proof_system::CurveType::SECP256K1) {
+            result = G1::secp256k1_ecdsa_mul(public_key, u1, u2);
+        } else {
+            result = G1::batch_mul({ G1::one(ctx), public_key }, { u1, u2 });
+        }
+        result.x.self_reduce();
+
+        // transfer Fq value x to an Fr element and reduce mod r
+        Fr result_mod_r(ctx, 0);
+        result_mod_r.binary_basis_limbs[0].element = result.x.binary_basis_limbs[0].element;
+        result_mod_r.binary_basis_limbs[1].element = result.x.binary_basis_limbs[1].element;
+        result_mod_r.binary_basis_limbs[2].element = result.x.binary_basis_limbs[2].element;
+        result_mod_r.binary_basis_limbs[3].element = result.x.binary_basis_limbs[3].element;
+        result_mod_r.binary_basis_limbs[0].maximum_value = result.x.binary_basis_limbs[0].maximum_value;
+        result_mod_r.binary_basis_limbs[1].maximum_value = result.x.binary_basis_limbs[1].maximum_value;
+        result_mod_r.binary_basis_limbs[2].maximum_value = result.x.binary_basis_limbs[2].maximum_value;
+        result_mod_r.binary_basis_limbs[3].maximum_value = result.x.binary_basis_limbs[3].maximum_value;
+
+        result_mod_r.prime_basis_limb = result.x.prime_basis_limb;
+
+        result_mod_r.assert_is_in_field();
+
+        result_mod_r.binary_basis_limbs[0].element.assert_equal(r.binary_basis_limbs[0].element);
+        result_mod_r.binary_basis_limbs[1].element.assert_equal(r.binary_basis_limbs[1].element);
+        result_mod_r.binary_basis_limbs[2].element.assert_equal(r.binary_basis_limbs[2].element);
+        result_mod_r.binary_basis_limbs[3].element.assert_equal(r.binary_basis_limbs[3].element);
+        result_mod_r.prime_basis_limb.assert_equal(r.prime_basis_limb);
+        return bool_t<Composer>(ctx, true);
     }
-    result.x.self_reduce();
-
-    // transfer Fq value x to an Fr element and reduce mod r
-    Fr result_mod_r(ctx, 0);
-    result_mod_r.binary_basis_limbs[0].element = result.x.binary_basis_limbs[0].element;
-    result_mod_r.binary_basis_limbs[1].element = result.x.binary_basis_limbs[1].element;
-    result_mod_r.binary_basis_limbs[2].element = result.x.binary_basis_limbs[2].element;
-    result_mod_r.binary_basis_limbs[3].element = result.x.binary_basis_limbs[3].element;
-    result_mod_r.binary_basis_limbs[0].maximum_value = result.x.binary_basis_limbs[0].maximum_value;
-    result_mod_r.binary_basis_limbs[1].maximum_value = result.x.binary_basis_limbs[1].maximum_value;
-    result_mod_r.binary_basis_limbs[2].maximum_value = result.x.binary_basis_limbs[2].maximum_value;
-    result_mod_r.binary_basis_limbs[3].maximum_value = result.x.binary_basis_limbs[3].maximum_value;
-
-    result_mod_r.prime_basis_limb = result.x.prime_basis_limb;
-
-    result_mod_r.assert_is_in_field();
-
-    result_mod_r.binary_basis_limbs[0].element.assert_equal(r.binary_basis_limbs[0].element);
-    result_mod_r.binary_basis_limbs[1].element.assert_equal(r.binary_basis_limbs[1].element);
-    result_mod_r.binary_basis_limbs[2].element.assert_equal(r.binary_basis_limbs[2].element);
-    result_mod_r.binary_basis_limbs[3].element.assert_equal(r.binary_basis_limbs[3].element);
-    result_mod_r.prime_basis_limb.assert_equal(r.prime_basis_limb);
-    return bool_t<Composer>(ctx, true);
-}
 
 /**
  * @brief Verify ECDSA signature. Returns 0 if signature fails (i.e. does not produce unsatisfiable constraints)