Skip to content

Commit

Permalink
Merge #755: Use Into<Message> in signing api
Browse files Browse the repository at this point in the history
ec0a69f Use Into<Message> in signing api (Liam Aharon)

Pull request description:

  Closes #700 (also see rust-bitcoin/rust-bitcoin#2821).

  Unrelated question while I have the authors attention: the schnorr apis (e.g. https://github.com/liamaharon/rust-secp256k1/blob/9afbf5111113ce84ff6f3b52f37c60554af2c283/secp256k1-sys/src/lib.rs#L81-L82) accepts a param named `msg32`, then directly after `msg_len`. I find it confusing since I would assume from the name `msg32` it must be 32 bytes. Should those instances be renamed `msg` if it is indeed variable len?

ACKs for top commit:
  tcharding:
    ACK ec0a69f
  apoelstra:
    ACK ec0a69f; successfully ran local tests; I wonder if we should do the same thing with SecretKey

Tree-SHA512: 2c3d9987d41f15ac91b0a77595b4d238cd52d9484a461f7541b2fd2e03b86f908ae7b34c76c1239501eb67246ba336161be1c6ae41d7f84122e9c003a3ade7d8
  • Loading branch information
apoelstra committed Oct 23, 2024
2 parents 36be55f + ec0a69f commit ac7c74a
Show file tree
Hide file tree
Showing 7 changed files with 99 additions and 92 deletions.
4 changes: 2 additions & 2 deletions 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(msg, &sig, &pubkey).is_ok())
}

fn sign<C: Signing>(
Expand All @@ -26,7 +26,7 @@ fn sign<C: Signing>(
let msg = sha256::Hash::hash(msg);
let msg = Message::from_digest_slice(msg.as_ref())?;
let seckey = SecretKey::from_slice(&seckey)?;
Ok(secp.sign_ecdsa(&msg, &seckey))
Ok(secp.sign_ecdsa(msg, &seckey))
}

fn main() {
Expand Down
4 changes: 2 additions & 2 deletions examples/sign_verify_recovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ fn recover<C: Verification>(
let id = ecdsa::RecoveryId::try_from(i32::from(recovery_id))?;
let sig = ecdsa::RecoverableSignature::from_compact(&sig, id)?;

secp.recover_ecdsa(&msg, &sig)
secp.recover_ecdsa(msg, &sig)
}

fn sign_recovery<C: Signing>(
Expand All @@ -26,7 +26,7 @@ fn sign_recovery<C: Signing>(
let msg = sha256::Hash::hash(msg);
let msg = Message::from_digest_slice(msg.as_ref())?;
let seckey = SecretKey::from_slice(&seckey)?;
Ok(secp.sign_ecdsa_recoverable(&msg, &seckey))
Ok(secp.sign_ecdsa_recoverable(msg, &seckey))
}

fn main() {
Expand Down
14 changes: 7 additions & 7 deletions no_std_test/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,12 @@ fn start(_argc: isize, _argv: *const *const u8) -> isize {
let public_key = PublicKey::from_secret_key(&secp, &secret_key);
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());
let sig = secp.sign_ecdsa(message, &secret_key);
assert!(secp.verify_ecdsa(message, &sig, &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_eq!(public_key, secp.recover_ecdsa(&message, &rec_sig).unwrap());
let rec_sig = secp.sign_ecdsa_recoverable(message, &secret_key);
assert!(secp.verify_ecdsa(message, &rec_sig.to_standard(), &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();
assert_eq!(rec_sig, new_rec_sig);
Expand All @@ -121,8 +121,8 @@ fn start(_argc: isize, _argv: *const *const u8) -> isize {
let public_key = PublicKey::from_secret_key(&secp_alloc, &secret_key);
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());
let sig = secp_alloc.sign_ecdsa(message, &secret_key);
assert!(secp_alloc.verify_ecdsa(message, &sig, &public_key).is_ok());
unsafe { libc::printf("Verified alloc Successfully!\n\0".as_ptr() as _) };
}

Expand Down
25 changes: 14 additions & 11 deletions src/ecdsa/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ impl Signature {
/// The signature must be normalized or verification will fail (see [`Signature::normalize_s`]).
#[inline]
#[cfg(feature = "global-context")]
pub fn verify(&self, msg: &Message, pk: &PublicKey) -> Result<(), Error> {
pub fn verify(&self, msg: impl Into<Message>, pk: &PublicKey) -> Result<(), Error> {
SECP256K1.verify_ecdsa(msg, self, pk)
}
}
Expand Down Expand Up @@ -243,10 +243,11 @@ impl<'de> serde::Deserialize<'de> for Signature {
impl<C: Signing> Secp256k1<C> {
fn sign_ecdsa_with_noncedata_pointer(
&self,
msg: &Message,
msg: impl Into<Message>,
sk: &SecretKey,
noncedata: Option<&[u8; 32]>,
) -> Signature {
let msg = msg.into();
unsafe {
let mut ret = ffi::Signature::new();
let noncedata_ptr = match noncedata {
Expand All @@ -272,7 +273,7 @@ impl<C: Signing> Secp256k1<C> {

/// Constructs a signature for `msg` using the secret key `sk` and RFC6979 nonce
/// Requires a signing-capable context.
pub fn sign_ecdsa(&self, msg: &Message, sk: &SecretKey) -> Signature {
pub fn sign_ecdsa(&self, msg: impl Into<Message>, sk: &SecretKey) -> Signature {
self.sign_ecdsa_with_noncedata_pointer(msg, sk, None)
}

Expand All @@ -283,7 +284,7 @@ impl<C: Signing> Secp256k1<C> {
/// Requires a signing-capable context.
pub fn sign_ecdsa_with_noncedata(
&self,
msg: &Message,
msg: impl Into<Message>,
sk: &SecretKey,
noncedata: &[u8; 32],
) -> Signature {
Expand All @@ -292,13 +293,14 @@ impl<C: Signing> Secp256k1<C> {

fn sign_grind_with_check(
&self,
msg: &Message,
msg: impl Into<Message>,
sk: &SecretKey,
check: impl Fn(&ffi::Signature) -> bool,
) -> Signature {
let mut entropy_p: *const ffi::types::c_void = ptr::null();
let mut counter: u32 = 0;
let mut extra_entropy = [0u8; 32];
let msg = msg.into();
loop {
unsafe {
let mut ret = ffi::Signature::new();
Expand Down Expand Up @@ -338,7 +340,7 @@ impl<C: Signing> Secp256k1<C> {
/// Requires a signing capable context.
pub fn sign_ecdsa_grind_r(
&self,
msg: &Message,
msg: impl Into<Message>,
sk: &SecretKey,
bytes_to_grind: usize,
) -> Signature {
Expand All @@ -352,7 +354,7 @@ impl<C: Signing> Secp256k1<C> {
/// signature implementation of bitcoin core. In average, this function
/// will perform two signing operations.
/// Requires a signing capable context.
pub fn sign_ecdsa_low_r(&self, msg: &Message, sk: &SecretKey) -> Signature {
pub fn sign_ecdsa_low_r(&self, msg: impl Into<Message>, sk: &SecretKey) -> Signature {
self.sign_grind_with_check(msg, sk, compact_sig_has_zero_first_bit)
}
}
Expand All @@ -372,20 +374,21 @@ impl<C: Verification> Secp256k1<C> {
/// # let (secret_key, public_key) = secp.generate_keypair(&mut rand::thread_rng());
/// #
/// 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(()));
/// let sig = secp.sign_ecdsa(message, &secret_key);
/// assert_eq!(secp.verify_ecdsa(message, &sig, &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(message, &sig, &public_key), Err(Error::IncorrectSignature));
/// # }
/// ```
#[inline]
pub fn verify_ecdsa(
&self,
msg: &Message,
msg: impl Into<Message>,
sig: &Signature,
pk: &PublicKey,
) -> Result<(), Error> {
let msg = msg.into();
unsafe {
if ffi::secp256k1_ecdsa_verify(
self.ctx.as_ptr(),
Expand Down
46 changes: 24 additions & 22 deletions src/ecdsa/recovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ impl RecoverableSignature {
/// verify-capable context.
#[inline]
#[cfg(feature = "global-context")]
pub fn recover(&self, msg: &Message) -> Result<key::PublicKey, Error> {
pub fn recover(&self, msg: impl Into<Message>) -> Result<key::PublicKey, Error> {
crate::SECP256K1.recover_ecdsa(msg, self)
}
}
Expand All @@ -154,10 +154,11 @@ impl From<ffi::RecoverableSignature> for RecoverableSignature {
impl<C: Signing> Secp256k1<C> {
fn sign_ecdsa_recoverable_with_noncedata_pointer(
&self,
msg: &Message,
msg: impl Into<Message>,
sk: &key::SecretKey,
noncedata_ptr: *const super_ffi::types::c_void,
) -> RecoverableSignature {
let msg = msg.into();
let mut ret = ffi::RecoverableSignature::new();
unsafe {
// We can assume the return value because it's not possible to construct
Expand All @@ -182,7 +183,7 @@ impl<C: Signing> Secp256k1<C> {
/// Requires a signing-capable context.
pub fn sign_ecdsa_recoverable(
&self,
msg: &Message,
msg: impl Into<Message>,
sk: &key::SecretKey,
) -> RecoverableSignature {
self.sign_ecdsa_recoverable_with_noncedata_pointer(msg, sk, ptr::null())
Expand All @@ -195,7 +196,7 @@ impl<C: Signing> Secp256k1<C> {
/// Requires a signing-capable context.
pub fn sign_ecdsa_recoverable_with_noncedata(
&self,
msg: &Message,
msg: impl Into<Message>,
sk: &key::SecretKey,
noncedata: &[u8; 32],
) -> RecoverableSignature {
Expand All @@ -209,9 +210,10 @@ impl<C: Verification> Secp256k1<C> {
/// `msg`. Requires a verify-capable context.
pub fn recover_ecdsa(
&self,
msg: &Message,
msg: impl Into<Message>,
sig: &RecoverableSignature,
) -> Result<key::PublicKey, Error> {
let msg = msg.into();
unsafe {
let mut pk = super_ffi::PublicKey::new();
if ffi::secp256k1_ecdsa_recover(
Expand Down Expand Up @@ -252,15 +254,15 @@ mod tests {
let (sk, pk) = full.generate_keypair(&mut rand::thread_rng());

// Try signing
assert_eq!(sign.sign_ecdsa_recoverable(&msg, &sk), full.sign_ecdsa_recoverable(&msg, &sk));
let sigr = full.sign_ecdsa_recoverable(&msg, &sk);
assert_eq!(sign.sign_ecdsa_recoverable(msg, &sk), full.sign_ecdsa_recoverable(msg, &sk));
let sigr = full.sign_ecdsa_recoverable(msg, &sk);

// Try pk recovery
assert!(vrfy.recover_ecdsa(&msg, &sigr).is_ok());
assert!(full.recover_ecdsa(&msg, &sigr).is_ok());
assert!(vrfy.recover_ecdsa(msg, &sigr).is_ok());
assert!(full.recover_ecdsa(msg, &sigr).is_ok());

assert_eq!(vrfy.recover_ecdsa(&msg, &sigr), full.recover_ecdsa(&msg, &sigr));
assert_eq!(full.recover_ecdsa(&msg, &sigr), Ok(pk));
assert_eq!(vrfy.recover_ecdsa(msg, &sigr), full.recover_ecdsa(msg, &sigr));
assert_eq!(full.recover_ecdsa(msg, &sigr), Ok(pk));
}

#[test]
Expand All @@ -280,7 +282,7 @@ mod tests {
let sk = SecretKey::from_slice(&ONE).unwrap();
let msg = Message::from_digest_slice(&ONE).unwrap();

let sig = s.sign_ecdsa_recoverable(&msg, &sk);
let sig = s.sign_ecdsa_recoverable(msg, &sk);

assert_eq!(Ok(sig), RecoverableSignature::from_compact(&[
0x66, 0x73, 0xff, 0xad, 0x21, 0x47, 0x74, 0x1f,
Expand All @@ -306,7 +308,7 @@ mod tests {
let msg = Message::from_digest_slice(&ONE).unwrap();
let noncedata = [42u8; 32];

let sig = s.sign_ecdsa_recoverable_with_noncedata(&msg, &sk, &noncedata);
let sig = s.sign_ecdsa_recoverable_with_noncedata(msg, &sk, &noncedata);

assert_eq!(Ok(sig), RecoverableSignature::from_compact(&[
0xb5, 0x0b, 0xb6, 0x79, 0x5f, 0x31, 0x74, 0x8a,
Expand All @@ -331,14 +333,14 @@ mod tests {

let (sk, pk) = s.generate_keypair(&mut rand::thread_rng());

let sigr = s.sign_ecdsa_recoverable(&msg, &sk);
let sigr = s.sign_ecdsa_recoverable(msg, &sk);
let sig = sigr.to_standard();

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(msg, &sig, &pk), Err(Error::IncorrectSignature));

let recovered_key = s.recover_ecdsa(&msg, &sigr).unwrap();
let recovered_key = s.recover_ecdsa(msg, &sigr).unwrap();
assert!(recovered_key != pk);
}

Expand All @@ -353,9 +355,9 @@ mod tests {

let (sk, pk) = s.generate_keypair(&mut rand::thread_rng());

let sig = s.sign_ecdsa_recoverable(&msg, &sk);
let sig = s.sign_ecdsa_recoverable(msg, &sk);

assert_eq!(s.recover_ecdsa(&msg, &sig), Ok(pk));
assert_eq!(s.recover_ecdsa(msg, &sig), Ok(pk));
}

#[test]
Expand All @@ -371,9 +373,9 @@ mod tests {

let (sk, pk) = s.generate_keypair(&mut rand::thread_rng());

let sig = s.sign_ecdsa_recoverable_with_noncedata(&msg, &sk, &noncedata);
let sig = s.sign_ecdsa_recoverable_with_noncedata(msg, &sk, &noncedata);

assert_eq!(s.recover_ecdsa(&msg, &sig), Ok(pk));
assert_eq!(s.recover_ecdsa(msg, &sig), Ok(pk));
}

#[test]
Expand All @@ -386,10 +388,10 @@ mod tests {

// Zero is not a valid sig
let sig = RecoverableSignature::from_compact(&[0; 64], RecoveryId::Zero).unwrap();
assert_eq!(s.recover_ecdsa(&msg, &sig), Err(Error::InvalidSignature));
assert_eq!(s.recover_ecdsa(msg, &sig), Err(Error::InvalidSignature));
// ...but 111..111 is
let sig = RecoverableSignature::from_compact(&[1; 64], RecoveryId::Zero).unwrap();
assert!(s.recover_ecdsa(&msg, &sig).is_ok());
assert!(s.recover_ecdsa(msg, &sig).is_ok());
}

#[test]
Expand Down
6 changes: 4 additions & 2 deletions src/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,9 @@ impl SecretKey {
/// Constructs an ECDSA signature for `msg` using the global [`SECP256K1`] context.
#[inline]
#[cfg(feature = "global-context")]
pub fn sign_ecdsa(&self, msg: Message) -> ecdsa::Signature { SECP256K1.sign_ecdsa(&msg, self) }
pub fn sign_ecdsa(&self, msg: impl Into<Message>) -> ecdsa::Signature {
SECP256K1.sign_ecdsa(msg, self)
}

/// Returns the [`Keypair`] for this [`SecretKey`].
///
Expand Down Expand Up @@ -737,7 +739,7 @@ impl PublicKey {
pub fn verify<C: Verification>(
&self,
secp: &Secp256k1<C>,
msg: &Message,
msg: impl Into<Message>,
sig: &ecdsa::Signature,
) -> Result<(), Error> {
secp.verify_ecdsa(msg, sig, self)
Expand Down
Loading

0 comments on commit ac7c74a

Please sign in to comment.