fix(ecdsa): correct short weierstrass curve eqn (#567)

Co-authored-by: codygunton <codygunton@gmail.com>

cpp/.aztec-packages-commit

@@ -1 +1 @@
-3e16992198189112739e3710860e7d7717366108
+master

cpp/src/barretenberg/stdlib/encryption/ecdsa/ecdsa.test.cpp

@@ -1,6 +1,7 @@
 #include "../../primitives/bigfield/bigfield.hpp"
 #include "../../primitives/biggroup/biggroup.hpp"
 #include "../../primitives/curves/secp256k1.hpp"
+#include "../../primitives/curves/secp256r1.hpp"
 #include "ecdsa.hpp"
 #include "barretenberg/crypto/ecdsa/ecdsa.hpp"
 #include "barretenberg/common/test.hpp"
@@ -11,6 +12,7 @@ using namespace proof_system::plonk;
 namespace test_stdlib_ecdsa {
 using Composer = proof_system::UltraCircuitBuilder;
 using curve = stdlib::secp256k1<Composer>;
+using curveR1 = stdlib::secp256r1<Composer>;
 
 TEST(stdlib_ecdsa, verify_signature)
 {
@@ -55,6 +57,49 @@ TEST(stdlib_ecdsa, verify_signature)
     EXPECT_EQ(proof_result, true);
 }
 
+TEST(stdlib_ecdsa, verify_r1_signature)
+{
+    Composer composer = Composer();
+
+    std::string message_string = "Instructions unclear, ask again later.";
+
+    crypto::ecdsa::key_pair<curveR1::fr, curveR1::g1> account;
+    account.private_key = curveR1::fr::random_element();
+    account.public_key = curveR1::g1::one * account.private_key;
+
+    crypto::ecdsa::signature signature =
+        crypto::ecdsa::construct_signature<Sha256Hasher, curveR1::fq, curveR1::fr, curveR1::g1>(message_string,
+                                                                                                account);
+
+    bool first_result = crypto::ecdsa::verify_signature<Sha256Hasher, curveR1::fq, curveR1::fr, curveR1::g1>(
+        message_string, account.public_key, signature);
+    EXPECT_EQ(first_result, true);
+
+    curveR1::g1_bigfr_ct public_key = curveR1::g1_bigfr_ct::from_witness(&composer, account.public_key);
+
+    std::vector<uint8_t> rr(signature.r.begin(), signature.r.end());
+    std::vector<uint8_t> ss(signature.s.begin(), signature.s.end());
+    uint8_t vv = signature.v;
+
+    stdlib::ecdsa::signature<Composer> sig{ curveR1::byte_array_ct(&composer, rr),
+                                            curveR1::byte_array_ct(&composer, ss),
+                                            stdlib::uint8<Composer>(&composer, vv) };
+
+    curveR1::byte_array_ct message(&composer, message_string);
+
+    curveR1::bool_ct signature_result =
+        stdlib::ecdsa::verify_signature<Composer, curveR1, curveR1::fq_ct, curveR1::bigfr_ct, curveR1::g1_bigfr_ct>(
+            message, public_key, sig);
+
+    EXPECT_EQ(signature_result.get_value(), true);
+
+    std::cerr << "composer gates = " << composer.get_num_gates() << std::endl;
+    benchmark_info(
+        Composer::NAME_STRING, "ECDSA", "Signature Verification Test", "Gate Count", composer.get_num_gates());
+    bool proof_result = composer.check_circuit();
+    EXPECT_EQ(proof_result, true);
+}
+
 TEST(stdlib_ecdsa, verify_signature_noassert_succeed)
 {
     Composer composer = Composer();

cpp/src/barretenberg/stdlib/encryption/ecdsa/ecdsa_impl.hpp

@@ -81,11 +81,13 @@ bool_t<Composer> verify_signature(const stdlib::byte_array<Composer>& message,
     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.
-    if constexpr (HasPlookup<Composer>) {
-        ASSERT(Curve::type == proof_system::CurveType::SECP256K1);
-        public_key.validate_on_curve();
+    // 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 });
@@ -155,10 +157,12 @@ bool_t<Composer> verify_signature_prehashed_message_noassert(const stdlib::byte_
     Fr u1 = z / s;
     Fr u2 = r / s;
 
+    public_key.validate_on_curve();
+
     G1 result;
-    if constexpr (HasPlookup<Composer>) {
-        ASSERT(Curve::type == proof_system::CurveType::SECP256K1);
-        public_key.validate_on_curve();
+    // 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 });

cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup.hpp

@@ -55,7 +55,7 @@ template <class Composer, class Fq, class Fr, class NativeGroup> class element {
         } else {
             Fq a(get_context(), uint256_t(NativeGroup::curve_a));
             // we validate y^2 = x^3 + ax + b by setting "fix_remainder_zero = true" when calling mult_madd
-            Fq::mult_madd({ x.sqr(), x, y }, { -x, a, y }, { b }, true);
+            Fq::mult_madd({ x.sqr(), x, y }, { x, a, -y }, { b }, true);
         }
     }