Replace _assign with _tweak

The key methods `add_assign`, `add_expr_assign`, and `mul_assign` are
cumbersome to use because a local variable that uses these methods
changes meaning but keeps the same identifier. It would be more useful
if we had methods that consumed `self` and returned a new key.

Observe also that these to methods are for adding/multiplying a key by a
tweak, rename the methods appropriately.

Add methods `add_tweak`, `add_expr_tweak`, and `mul_tweak` to the
`SecretKey` and `PublicKey` type. Deprecate `add_assign`,
`add_expr_assign`, and `mul_assign`.
This commit is contained in:
Tobin Harding 2022-04-06 14:05:04 +10:00 committed by Tobin C. Harding
parent aab77b16c2
commit b9d08db8eb
2 changed files with 125 additions and 50 deletions

View File

@ -228,47 +228,67 @@ impl SecretKey {
} }
} }
#[inline]
/// Adds one secret key to another, modulo the curve order. /// Adds one secret key to another, modulo the curve order.
/// ///
/// # Errors /// # Errors
/// ///
/// Returns an error if the resulting key would be invalid. /// Returns an error if the resulting key would be invalid.
pub fn add_assign( #[inline]
&mut self, #[deprecated(since = "0.23.0", note = "Use add_tweak instead")]
other: &Scalar, pub fn add_assign(&mut self, other: &Scalar) -> Result<(), Error> {
) -> Result<(), Error> { *self = self.add_tweak(other)?;
Ok(())
}
/// Tweaks a [`SecretKey`] by adding `tweak` modulo the curve order.
///
/// # Errors
///
/// Returns an error if the resulting key would be invalid or if the tweak was not a 32-byte
/// length slice.
#[inline]
pub fn add_tweak(mut self, tweak: &Scalar) -> Result<SecretKey, Error> {
unsafe { unsafe {
if ffi::secp256k1_ec_seckey_tweak_add( if ffi::secp256k1_ec_seckey_tweak_add(
ffi::secp256k1_context_no_precomp, ffi::secp256k1_context_no_precomp,
self.as_mut_c_ptr(), self.as_mut_c_ptr(),
other.as_c_ptr(), tweak.as_c_ptr(),
) != 1 ) != 1
{ {
Err(Error::InvalidTweak) Err(Error::InvalidTweak)
} else { } else {
Ok(()) Ok(self)
} }
} }
} }
#[inline]
/// Multiplies one secret key by another, modulo the curve order. Will /// Multiplies one secret key by another, modulo the curve order. Will
/// return an error if the resulting key would be invalid. /// return an error if the resulting key would be invalid.
pub fn mul_assign( #[inline]
&mut self, #[deprecated(since = "0.23.0", note = "Use mul_tweak instead")]
other: &Scalar, pub fn mul_assign(&mut self, other: &Scalar) -> Result<(), Error> {
) -> Result<(), Error> { *self = self.mul_tweak(other)?;
Ok(())
}
/// Tweaks a [`SecretKey`] by multiplying by `tweak` modulo the curve order.
///
/// # Errors
///
/// Returns an error if the resulting key would be invalid or if the tweak was not a 32-byte
/// length slice.
#[inline]
pub fn mul_tweak(mut self, tweak: &Scalar) -> Result<SecretKey, Error> {
unsafe { unsafe {
if ffi::secp256k1_ec_seckey_tweak_mul( if ffi::secp256k1_ec_seckey_tweak_mul(
ffi::secp256k1_context_no_precomp, ffi::secp256k1_context_no_precomp,
self.as_mut_c_ptr(), self.as_mut_c_ptr(),
other.as_c_ptr(), tweak.as_c_ptr(),
) != 1 ) != 1
{ {
Err(Error::InvalidTweak) Err(Error::InvalidTweak)
} else { } else {
Ok(()) Ok(self)
} }
} }
} }
@ -490,40 +510,72 @@ impl PublicKey {
} }
} }
#[inline]
/// Adds `other * G` to `self` in place. /// Adds `other * G` to `self` in place.
/// ///
/// # Errors /// # Errors
/// ///
/// Returns an error if the resulting key would be invalid. /// Returns an error if the resulting key would be invalid.
#[inline]
#[deprecated(since = "0.23.0", note = "Use add_exp_tweak instead")]
pub fn add_exp_assign<C: Verification>( pub fn add_exp_assign<C: Verification>(
&mut self, &mut self,
secp: &Secp256k1<C>, secp: &Secp256k1<C>,
other: &Scalar other: &Scalar
) -> Result<(), Error> { ) -> Result<(), Error> {
unsafe { *self = self.add_exp_tweak(secp, other)?;
if ffi::secp256k1_ec_pubkey_tweak_add(secp.ctx, &mut self.0, other.as_c_ptr()) == 1 {
Ok(()) Ok(())
}
/// Tweaks a [`PublicKey`] by adding `tweak * G` modulo the curve order.
///
/// # Errors
///
/// Returns an error if the resulting key would be invalid.
#[inline]
pub fn add_exp_tweak<C: Verification>(
mut self,
secp: &Secp256k1<C>,
tweak: &Scalar
) -> Result<PublicKey, Error> {
unsafe {
if ffi::secp256k1_ec_pubkey_tweak_add(secp.ctx, &mut self.0, tweak.as_c_ptr()) == 1 {
Ok(self)
} else { } else {
Err(Error::InvalidTweak) Err(Error::InvalidTweak)
} }
} }
} }
#[inline]
/// Muliplies the public key in place by the scalar `other`. /// Muliplies the public key in place by the scalar `other`.
/// ///
/// # Errors /// # Errors
/// ///
/// Returns an error if the resulting key would be invalid. /// Returns an error if the resulting key would be invalid.
#[deprecated(since = "0.23.0", note = "Use mul_tweak instead")]
#[inline]
pub fn mul_assign<C: Verification>( pub fn mul_assign<C: Verification>(
&mut self, &mut self,
secp: &Secp256k1<C>, secp: &Secp256k1<C>,
other: &Scalar, other: &Scalar,
) -> Result<(), Error> { ) -> Result<(), Error> {
*self = self.mul_tweak(secp, other)?;
Ok(())
}
/// Tweaks a [`PublicKey`] by multiplying by `tweak` modulo the curve order.
///
/// # Errors
///
/// Returns an error if the resulting key would be invalid.
#[inline]
pub fn mul_tweak<C: Verification>(
mut self,
secp: &Secp256k1<C>,
other: &Scalar,
) -> Result<PublicKey, Error> {
unsafe { unsafe {
if ffi::secp256k1_ec_pubkey_tweak_mul(secp.ctx, &mut self.0, other.as_c_ptr()) == 1 { if ffi::secp256k1_ec_pubkey_tweak_mul(secp.ctx, &mut self.0, other.as_c_ptr()) == 1 {
Ok(()) Ok(self)
} else { } else {
Err(Error::InvalidTweak) Err(Error::InvalidTweak)
} }
@ -1474,6 +1526,8 @@ pub mod serde_keypair {
#[cfg(test)] #[cfg(test)]
#[allow(unused_imports)] #[allow(unused_imports)]
mod test { mod test {
use super::*;
use core::str::FromStr; use core::str::FromStr;
#[cfg(any(feature = "alloc", feature = "std"))] #[cfg(any(feature = "alloc", feature = "std"))]
@ -1738,45 +1792,66 @@ mod test {
} }
#[test] #[test]
#[cfg(any(feature = "alloc", feature = "std"))] #[cfg(feature = "rand-std")]
fn test_addition() { fn tweak_add_arbitrary_data() {
let s = Secp256k1::new(); let s = Secp256k1::new();
let (mut sk1, mut pk1) = s.generate_keypair(&mut thread_rng()); let (sk, pk) = s.generate_keypair(&mut thread_rng());
let (mut sk2, mut pk2) = s.generate_keypair(&mut thread_rng()); assert_eq!(PublicKey::from_secret_key(&s, &sk), pk); // Sanity check.
let scalar1 = Scalar::from(sk1);
let scalar2 = Scalar::from(sk1);
assert_eq!(PublicKey::from_secret_key(&s, &sk1), pk1); // TODO: This would be better tested with a _lot_ of different tweaks.
assert!(sk1.add_assign(&scalar2).is_ok()); let tweak = Scalar::random();
assert!(pk1.add_exp_assign(&s, &scalar2).is_ok());
assert_eq!(PublicKey::from_secret_key(&s, &sk1), pk1);
assert_eq!(PublicKey::from_secret_key(&s, &sk2), pk2); let tweaked_sk = sk.add_tweak(&tweak).unwrap();
assert!(sk2.add_assign(&scalar1).is_ok()); assert_ne!(sk, tweaked_sk); // Make sure we did something.
assert!(pk2.add_exp_assign(&s, &scalar1).is_ok()); let tweaked_pk = pk.add_exp_tweak(&s, &tweak).unwrap();
assert_eq!(PublicKey::from_secret_key(&s, &sk2), pk2); assert_ne!(pk, tweaked_pk);
assert_eq!(PublicKey::from_secret_key(&s, &tweaked_sk), tweaked_pk);
} }
#[test] #[test]
#[cfg(any(feature = "alloc", feature = "std"))] #[cfg(any(feature = "alloc", feature = "std"))]
fn test_multiplication() { fn tweak_add_zero() {
let s = Secp256k1::new(); let s = Secp256k1::new();
let (mut sk1, mut pk1) = s.generate_keypair(&mut thread_rng()); let (sk, pk) = s.generate_keypair(&mut thread_rng());
let (mut sk2, mut pk2) = s.generate_keypair(&mut thread_rng());
let scalar1 = Scalar::from(sk1);
let scalar2 = Scalar::from(sk1);
assert_eq!(PublicKey::from_secret_key(&s, &sk1), pk1); let tweak = Scalar::ZERO;
assert!(sk1.mul_assign(&scalar2).is_ok());
assert!(pk1.mul_assign(&s, &scalar2).is_ok());
assert_eq!(PublicKey::from_secret_key(&s, &sk1), pk1);
assert_eq!(PublicKey::from_secret_key(&s, &sk2), pk2); let tweaked_sk = sk.add_tweak(&tweak).unwrap();
assert!(sk2.mul_assign(&scalar1).is_ok()); assert_eq!(sk, tweaked_sk); // Tweak by zero does nothing.
assert!(pk2.mul_assign(&s, &scalar1).is_ok()); let tweaked_pk = pk.add_exp_tweak(&s, &tweak).unwrap();
assert_eq!(PublicKey::from_secret_key(&s, &sk2), pk2); assert_eq!(pk, tweaked_pk);
}
#[test]
#[cfg(feature = "rand-std")]
fn tweak_mul_arbitrary_data() {
let s = Secp256k1::new();
let (sk, pk) = s.generate_keypair(&mut thread_rng());
assert_eq!(PublicKey::from_secret_key(&s, &sk), pk); // Sanity check.
// TODO: This would be better tested with a _lot_ of different tweaks.
let tweak = Scalar::random();
let tweaked_sk = sk.mul_tweak(&tweak).unwrap();
assert_ne!(sk, tweaked_sk); // Make sure we did something.
let tweaked_pk = pk.mul_tweak(&s, &tweak).unwrap();
assert_ne!(pk, tweaked_pk);
assert_eq!(PublicKey::from_secret_key(&s, &tweaked_sk), tweaked_pk);
}
#[test]
#[cfg(any(feature = "alloc", feature = "std"))]
fn tweak_mul_zero() {
let s = Secp256k1::new();
let (sk, _) = s.generate_keypair(&mut thread_rng());
let tweak = Scalar::ZERO;
assert!(sk.mul_tweak(&tweak).is_err())
} }
#[test] #[test]
@ -1882,7 +1957,7 @@ mod test {
fn create_pubkey_combine() { fn create_pubkey_combine() {
let s = Secp256k1::new(); let s = Secp256k1::new();
let (mut sk1, pk1) = s.generate_keypair(&mut thread_rng()); let (sk1, pk1) = s.generate_keypair(&mut thread_rng());
let (sk2, pk2) = s.generate_keypair(&mut thread_rng()); let (sk2, pk2) = s.generate_keypair(&mut thread_rng());
let sum1 = pk1.combine(&pk2); let sum1 = pk1.combine(&pk2);
@ -1891,8 +1966,8 @@ mod test {
assert!(sum2.is_ok()); assert!(sum2.is_ok());
assert_eq!(sum1, sum2); assert_eq!(sum1, sum2);
assert!(sk1.add_assign(&Scalar::from(sk2)).is_ok()); let tweaked = sk1.add_tweak(&Scalar::from(sk2)).unwrap();
let sksum = PublicKey::from_secret_key(&s, &sk1); let sksum = PublicKey::from_secret_key(&s, &tweaked);
assert_eq!(Ok(sksum), sum1); assert_eq!(Ok(sksum), sum1);
} }

View File

@ -339,7 +339,7 @@ pub enum Error {
InvalidSharedSecret, InvalidSharedSecret,
/// Bad recovery id. /// Bad recovery id.
InvalidRecoveryId, InvalidRecoveryId,
/// Invalid tweak for `add_*_assign` or `mul_*_assign`. /// Tried to add/multiply by an invalid tweak.
InvalidTweak, InvalidTweak,
/// Didn't pass enough memory to context creation with preallocated memory. /// Didn't pass enough memory to context creation with preallocated memory.
NotEnoughMemory, NotEnoughMemory,