Skip to content

Commit

Permalink
Reorder verify_ecdsa args to match the order of verify_schnorr
Browse files Browse the repository at this point in the history
When `verify_schnorr` was first introduced, it used parameters in the inuitive order: sig, msg, key. It was later noticed that `verify_ecdsa` had the same parameters but in a different order. Since both functions have been released, this will be a breaking change, but fixing this now will add consistency and remove any confusion for users in the future.
  • Loading branch information
shinghim committed Oct 14, 2024
1 parent 379e128 commit 0f9a513
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 31 deletions.
2 changes: 1 addition & 1 deletion examples/sign_verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>(
Expand Down
6 changes: 3 additions & 3 deletions no_std_test/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,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();
Expand All @@ -122,7 +122,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 _) };
}

Expand Down
8 changes: 4 additions & 4 deletions src/ecdsa/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ impl Signature {
#[inline]
#[cfg(feature = "global-context")]
pub fn verify(&self, msg: &Message, pk: &PublicKey) -> Result<(), Error> {
SECP256K1.verify_ecdsa(msg, self, pk)
SECP256K1.verify_ecdsa(self, msg, pk)
}
}

Expand Down Expand Up @@ -373,17 +373,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: &Message,
sig: &Signature,
msg: &Message,
pk: &PublicKey,
) -> Result<(), Error> {
unsafe {
Expand Down
2 changes: 1 addition & 1 deletion src/ecdsa/recovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ mod tests {

let msg = crate::random_32_bytes(&mut rand::thread_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);
Expand Down
2 changes: 1 addition & 1 deletion src/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -740,7 +740,7 @@ impl PublicKey {
msg: &Message,
sig: &ecdsa::Signature,
) -> Result<(), Error> {
secp.verify_ecdsa(msg, sig, self)
secp.verify_ecdsa(sig, msg, self)
}
}

Expand Down
42 changes: 21 additions & 21 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
//! # }
//! ```
//!
Expand Down Expand Up @@ -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());
//! # }
//! ```
//!
Expand Down Expand Up @@ -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());
//! # }
//! ```
//!
Expand Down Expand Up @@ -547,8 +547,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.
Expand Down Expand Up @@ -614,8 +614,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]
Expand All @@ -636,8 +636,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[..]);
Expand Down Expand Up @@ -750,13 +750,13 @@ mod tests {

let (sk, pk) = s.generate_keypair(&mut rand::thread_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);
Expand Down Expand Up @@ -798,9 +798,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(()));
}
}
}
Expand All @@ -820,7 +820,7 @@ mod tests {

let msg = crate::random_32_bytes(&mut rand::thread_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]
Expand Down Expand Up @@ -913,10 +913,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]
Expand Down Expand Up @@ -1000,7 +1000,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());
}
}

Expand Down Expand Up @@ -1045,7 +1045,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);
});
}
Expand Down

0 comments on commit 0f9a513

Please sign in to comment.