Merge rust-bitcoin/rust-secp256k1#751: Reorder verify_ecdsa args to match those of verify_schnorr

c051b145234177481dea4326566442fddc16537e Reorder verify_ecdsa args to match the order of verify_schnorr (Shing Him Ng)

Pull request description:

  The issue  (#617) seemed to have good support with 4 thumbs up, so this PR will reorder `verify_ecdsa` args to match those of `verify_schnorr`, which just swaps the place of `msg` and `sig`. This will break the API, but will potentially cause less confusion in the future

  Closes #617

ACKs for top commit:
  apoelstra:
    ACK c051b145234177481dea4326566442fddc16537e; successfully ran local tests
  tcharding:
    ACK c051b145234177481dea4326566442fddc16537e

Tree-SHA512: 0798baa48e3073aea2fc68195124bf1d9c99fcfff2306f931b89fe8a6d0a8157203098cf6e5979c6f3baa09d294e251d915ab55d6a49b34d186f61853667b057

Author: william2332-limf

examples/sign_verify.rs

@@ -15,7 +15,7 @@ fn verify<C: Verification>(
     let sig = ecdsa::Signature::from_compact(&sig)?;
     let pubkey = PublicKey::from_slice(&pubkey)?;
 
-    Ok(secp.verify_ecdsa(msg, &sig, &pubkey).is_ok())
+    Ok(secp.verify_ecdsa(&sig, msg, &pubkey).is_ok())
 }
 
 fn sign<C: Signing>(

no_std_test/src/main.rs

@@ -91,10 +91,10 @@ fn start(_argc: isize, _argv: *const *const u8) -> isize {
     let message = Message::from_digest_slice(&[0xab; 32]).expect("32 bytes");
 
     let sig = secp.sign_ecdsa(message, &secret_key);
-    assert!(secp.verify_ecdsa(message, &sig, &public_key).is_ok());
+    assert!(secp.verify_ecdsa(&sig, message, &public_key).is_ok());
 
     let rec_sig = secp.sign_ecdsa_recoverable(message, &secret_key);
-    assert!(secp.verify_ecdsa(message, &rec_sig.to_standard(), &public_key).is_ok());
+    assert!(secp.verify_ecdsa(&rec_sig.to_standard(), message, &public_key).is_ok());
     assert_eq!(public_key, secp.recover_ecdsa(message, &rec_sig).unwrap());
     let (rec_id, data) = rec_sig.serialize_compact();
     let new_rec_sig = ecdsa::RecoverableSignature::from_compact(&data, rec_id).unwrap();
@@ -118,7 +118,7 @@ fn start(_argc: isize, _argv: *const *const u8) -> isize {
         let message = Message::from_digest_slice(&[0xab; 32]).expect("32 bytes");
 
         let sig = secp_alloc.sign_ecdsa(message, &secret_key);
-        assert!(secp_alloc.verify_ecdsa(message, &sig, &public_key).is_ok());
+        assert!(secp_alloc.verify_ecdsa(&sig, message, &public_key).is_ok());
         unsafe { libc::printf("Verified alloc Successfully!\n\0".as_ptr() as _) };
     }
 

src/ecdsa/mod.rs

@@ -195,7 +195,7 @@ impl Signature {
     #[inline]
     #[cfg(feature = "global-context")]
     pub fn verify(&self, msg: impl Into<Message>, pk: &PublicKey) -> Result<(), Error> {
-        SECP256K1.verify_ecdsa(msg, self, pk)
+        SECP256K1.verify_ecdsa(self, msg, pk)
     }
 }
 
@@ -375,17 +375,17 @@ impl<C: Verification> Secp256k1<C> {
     /// #
     /// let message = Message::from_digest_slice(&[0xab; 32]).expect("32 bytes");
     /// let sig = secp.sign_ecdsa(message, &secret_key);
-    /// assert_eq!(secp.verify_ecdsa(message, &sig, &public_key), Ok(()));
+    /// assert_eq!(secp.verify_ecdsa(&sig, message, &public_key), Ok(()));
     ///
     /// let message = Message::from_digest_slice(&[0xcd; 32]).expect("32 bytes");
-    /// assert_eq!(secp.verify_ecdsa(message, &sig, &public_key), Err(Error::IncorrectSignature));
+    /// assert_eq!(secp.verify_ecdsa(&sig, message, &public_key), Err(Error::IncorrectSignature));
     /// # }
     /// ```
     #[inline]
     pub fn verify_ecdsa(
         &self,
-        msg: impl Into<Message>,
         sig: &Signature,
+        msg: impl Into<Message>,
         pk: &PublicKey,
     ) -> Result<(), Error> {
         let msg = msg.into();

src/ecdsa/recovery.rs

@@ -361,7 +361,7 @@ mod tests {
 
         let msg = crate::random_32_bytes(&mut rand::rng());
         let msg = Message::from_digest_slice(&msg).unwrap();
-        assert_eq!(s.verify_ecdsa(msg, &sig, &pk), Err(Error::IncorrectSignature));
+        assert_eq!(s.verify_ecdsa(&sig, msg, &pk), Err(Error::IncorrectSignature));
 
         let recovered_key = s.recover_ecdsa(msg, &sigr).unwrap();
         assert!(recovered_key != pk);

src/key.rs

@@ -743,7 +743,7 @@ impl PublicKey {
         msg: impl Into<Message>,
         sig: &ecdsa::Signature,
     ) -> Result<(), Error> {
-        secp.verify_ecdsa(msg, sig, self)
+        secp.verify_ecdsa(sig, msg, self)
     }
 }
 

src/lib.rs

@@ -39,7 +39,7 @@
 //! let message = Message::from_digest(digest.to_byte_array());
 //!
 //! let sig = secp.sign_ecdsa(message, &secret_key);
-//! assert!(secp.verify_ecdsa(message, &sig, &public_key).is_ok());
+//! assert!(secp.verify_ecdsa(&sig, message, &public_key).is_ok());
 //! # }
 //! ```
 //!
@@ -76,7 +76,7 @@
 //! let message = Message::from_digest(compute_hash(b"CSW is not Satoshi"));
 //!
 //! let sig = secp.sign_ecdsa(message, &secret_key);
-//! assert!(secp.verify_ecdsa(message, &sig, &public_key).is_ok());
+//! assert!(secp.verify_ecdsa(&sig, message, &public_key).is_ok());
 //! # }
 //! ```
 //!
@@ -115,7 +115,7 @@
 //! ]).expect("compact signatures are 64 bytes; DER signatures are 68-72 bytes");
 //!
 //! # #[cfg(not(secp256k1_fuzz))]
-//! assert!(secp.verify_ecdsa(message, &sig, &public_key).is_ok());
+//! assert!(secp.verify_ecdsa(&sig, message, &public_key).is_ok());
 //! # }
 //! ```
 //!
@@ -548,8 +548,8 @@ mod tests {
         let sig = full.sign_ecdsa(msg, &sk);
 
         // Try verifying
-        assert!(vrfy.verify_ecdsa(msg, &sig, &pk).is_ok());
-        assert!(full.verify_ecdsa(msg, &sig, &pk).is_ok());
+        assert!(vrfy.verify_ecdsa(&sig, msg, &pk).is_ok());
+        assert!(full.verify_ecdsa(&sig, msg, &pk).is_ok());
 
         // The following drop will have no effect; in fact, they will trigger a compiler
         // error because manually dropping a `ManuallyDrop` is almost certainly incorrect.
@@ -615,8 +615,8 @@ mod tests {
         let sig = full.sign_ecdsa(msg, &sk);
 
         // Try verifying
-        assert!(vrfy.verify_ecdsa(msg, &sig, &pk).is_ok());
-        assert!(full.verify_ecdsa(msg, &sig, &pk).is_ok());
+        assert!(vrfy.verify_ecdsa(&sig, msg, &pk).is_ok());
+        assert!(full.verify_ecdsa(&sig, msg, &pk).is_ok());
     }
 
     #[test]
@@ -637,8 +637,8 @@ mod tests {
         let sig = full.sign_ecdsa(msg, &sk);
 
         // Try verifying
-        assert!(vrfy.verify_ecdsa(msg, &sig, &pk).is_ok());
-        assert!(full.verify_ecdsa(msg, &sig, &pk).is_ok());
+        assert!(vrfy.verify_ecdsa(&sig, msg, &pk).is_ok());
+        assert!(full.verify_ecdsa(&sig, msg, &pk).is_ok());
 
         // Check that we can produce keys from slices with no precomputation
         let (pk_slice, sk_slice) = (&pk.serialize(), &sk[..]);
@@ -751,13 +751,13 @@ mod tests {
 
             let (sk, pk) = s.generate_keypair(&mut rand::rng());
             let sig = s.sign_ecdsa(msg, &sk);
-            assert_eq!(s.verify_ecdsa(msg, &sig, &pk), Ok(()));
+            assert_eq!(s.verify_ecdsa(&sig, msg, &pk), Ok(()));
             let noncedata_sig = s.sign_ecdsa_with_noncedata(msg, &sk, &noncedata);
-            assert_eq!(s.verify_ecdsa(msg, &noncedata_sig, &pk), Ok(()));
+            assert_eq!(s.verify_ecdsa(&noncedata_sig, msg, &pk), Ok(()));
             let low_r_sig = s.sign_ecdsa_low_r(msg, &sk);
-            assert_eq!(s.verify_ecdsa(msg, &low_r_sig, &pk), Ok(()));
+            assert_eq!(s.verify_ecdsa(&low_r_sig, msg, &pk), Ok(()));
             let grind_r_sig = s.sign_ecdsa_grind_r(msg, &sk, 1);
-            assert_eq!(s.verify_ecdsa(msg, &grind_r_sig, &pk), Ok(()));
+            assert_eq!(s.verify_ecdsa(&grind_r_sig, msg, &pk), Ok(()));
             let compact = sig.serialize_compact();
             if compact[0] < 0x80 {
                 assert_eq!(sig, low_r_sig);
@@ -799,9 +799,9 @@ mod tests {
                 let low_r_sig = s.sign_ecdsa_low_r(msg, &key);
                 let grind_r_sig = s.sign_ecdsa_grind_r(msg, &key, 1);
                 let pk = PublicKey::from_secret_key(&s, &key);
-                assert_eq!(s.verify_ecdsa(msg, &sig, &pk), Ok(()));
-                assert_eq!(s.verify_ecdsa(msg, &low_r_sig, &pk), Ok(()));
-                assert_eq!(s.verify_ecdsa(msg, &grind_r_sig, &pk), Ok(()));
+                assert_eq!(s.verify_ecdsa(&sig, msg, &pk), Ok(()));
+                assert_eq!(s.verify_ecdsa(&low_r_sig, msg, &pk), Ok(()));
+                assert_eq!(s.verify_ecdsa(&grind_r_sig, msg, &pk), Ok(()));
             }
         }
     }
@@ -821,7 +821,7 @@ mod tests {
 
         let msg = crate::random_32_bytes(&mut rand::rng());
         let msg = Message::from_digest(msg);
-        assert_eq!(s.verify_ecdsa(msg, &sig, &pk), Err(Error::IncorrectSignature));
+        assert_eq!(s.verify_ecdsa(&sig, msg, &pk), Err(Error::IncorrectSignature));
     }
 
     #[test]
@@ -914,10 +914,10 @@ mod tests {
         let msg = Message::from_digest(msg);
 
         // without normalization we expect this will fail
-        assert_eq!(secp.verify_ecdsa(msg, &sig, &pk), Err(Error::IncorrectSignature));
+        assert_eq!(secp.verify_ecdsa(&sig, msg, &pk), Err(Error::IncorrectSignature));
         // after normalization it should pass
         sig.normalize_s();
-        assert_eq!(secp.verify_ecdsa(msg, &sig, &pk), Ok(()));
+        assert_eq!(secp.verify_ecdsa(&sig, msg, &pk), Ok(()));
     }
 
     #[test]
@@ -1001,7 +1001,7 @@ mod tests {
 
         // Check usage as self
         let sig = SECP256K1.sign_ecdsa(msg, &sk);
-        assert!(SECP256K1.verify_ecdsa(msg, &sig, &pk).is_ok());
+        assert!(SECP256K1.verify_ecdsa(&sig, msg, &pk).is_ok());
     }
 }
 
@@ -1046,7 +1046,7 @@ mod benches {
         let sig = s.sign_ecdsa(msg, &sk);
 
         bh.iter(|| {
-            let res = s.verify_ecdsa(msg, &sig, &pk).unwrap();
+            let res = s.verify_ecdsa(&sig, msg, &pk).unwrap();
             black_box(res);
         });
     }