Skip to content

Commit

Permalink
Merge #781: feat: Create keys from owned array values
Browse files Browse the repository at this point in the history 
59cf7bd refactor: Create keys from owned array values (Christian Lewe)
e7eea32 feat: KeyPair::from_seckey_byte_array (Christian Lewe)
e723d80 refactor: Parse byte array instead of byte slice (Christian Lewe)

Pull request description:

  Construct KeyPair directly from [u8; 32]. I can change the parameter to &[u8; 32] depending on the discussion in #780.

ACKs for top commit:
  Kixunil:
    ACK 59cf7bd
  apoelstra:
    ACK 59cf7bd; successfully ran local tests; nice! the old function signatures were super weird. and the docs were wrong lol
  tcharding:
    ACK 59cf7bd

Tree-SHA512: 64e3026984c79b2491b83775bd50b203ad0723f97ac02b4aa84bc5d0e1135c7fabb660a7d242beaa3f2799f16b169c6bbe3f24e31668518b59ed01839bb86c69
  • Loading branch information
@apoelstra
apoelstra committed Feb 19, 2025
2 parents 0ecd2a2 + 59cf7bd commit ede62d5
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 44 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# Next

* Create keys from owned array values instead of from references [#781](https://github.com/rust-bitcoin/rust-secp256k1/pull/781)

# 0.30.0 - 2024-10-08

* Allow signing variable-length messages [#706](https://github.com/rust-bitcoin/rust-secp256k1/pull/706)
Expand Down
86 changes: 49 additions & 37 deletions src/key.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ impl str::FromStr for SecretKey {
fn from_str(s: &str) -> Result<SecretKey, Error> {
let mut res = [0u8; constants::SECRET_KEY_SIZE];
match from_hex(s, &mut res) {
Ok(constants::SECRET_KEY_SIZE) => SecretKey::from_byte_array(&res),
Ok(constants::SECRET_KEY_SIZE) => SecretKey::from_byte_array(res),
_ => Err(Error::InvalidSecretKey),
}
}
Expand All @@ -138,7 +138,7 @@ impl str::FromStr for SecretKey {
/// use secp256k1::{SecretKey, Secp256k1, PublicKey};
///
/// let secp = Secp256k1::new();
/// let secret_key = SecretKey::from_byte_array(&[0xcd; 32]).expect("32 bytes, within curve order");
/// let secret_key = SecretKey::from_byte_array([0xcd; 32]).expect("32 bytes, within curve order");
/// let public_key = PublicKey::from_secret_key(&secp, &secret_key);
/// # }
/// ```
Expand Down Expand Up @@ -175,10 +175,10 @@ impl str::FromStr for PublicKey {
Ok(constants::PUBLIC_KEY_SIZE) => {
let bytes: [u8; constants::PUBLIC_KEY_SIZE] =
res[0..constants::PUBLIC_KEY_SIZE].try_into().unwrap();
PublicKey::from_byte_array_compressed(&bytes)
PublicKey::from_byte_array_compressed(bytes)
}
Ok(constants::UNCOMPRESSED_PUBLIC_KEY_SIZE) =>
PublicKey::from_byte_array_uncompressed(&res),
PublicKey::from_byte_array_uncompressed(res),
_ => Err(Error::InvalidPublicKey),
}
}
Expand Down Expand Up @@ -223,7 +223,7 @@ impl SecretKey {
#[inline]
pub fn from_slice(data: &[u8]) -> Result<SecretKey, Error> {
match <[u8; constants::SECRET_KEY_SIZE]>::try_from(data) {
Ok(data) => Self::from_byte_array(&data),
Ok(data) => Self::from_byte_array(data),
Err(_) => Err(InvalidSecretKey),
}
}
Expand All @@ -234,18 +234,18 @@ impl SecretKey {
///
/// ```
/// use secp256k1::SecretKey;
/// let sk = SecretKey::from_byte_array(&[0xcd; 32]).expect("32 bytes, within curve order");
/// let sk = SecretKey::from_byte_array([0xcd; 32]).expect("32 bytes, within curve order");
/// ```
#[inline]
pub fn from_byte_array(data: &[u8; constants::SECRET_KEY_SIZE]) -> Result<SecretKey, Error> {
pub fn from_byte_array(data: [u8; constants::SECRET_KEY_SIZE]) -> Result<SecretKey, Error> {
unsafe {
if ffi::secp256k1_ec_seckey_verify(ffi::secp256k1_context_no_precomp, data.as_c_ptr())
== 0
{
return Err(InvalidSecretKey);
}
}
Ok(SecretKey(*data))
Ok(SecretKey(data))
}

/// Creates a new secret key using data from BIP-340 [`Keypair`].
Expand Down Expand Up @@ -373,7 +373,7 @@ impl SecretKey {
impl<T: ThirtyTwoByteHash> From<T> for SecretKey {
/// Converts a 32-byte hash directly to a secret key without error paths.
fn from(t: T) -> SecretKey {
SecretKey::from_byte_array(&t.into_32()).expect("failed to create secret key")
SecretKey::from_byte_array(t.into_32()).expect("failed to create secret key")
}
}

Expand Down Expand Up @@ -401,10 +401,10 @@ impl<'de> serde::Deserialize<'de> for SecretKey {
"a hex string representing 32 byte SecretKey",
))
} else {
let visitor = super::serde_util::Tuple32Visitor::new(
"raw 32 bytes SecretKey",
SecretKey::from_slice,
);
let visitor =
super::serde_util::Tuple32Visitor::new("raw 32 bytes SecretKey", |bytes| {
SecretKey::from_byte_array(bytes)
});
d.deserialize_tuple(constants::SECRET_KEY_SIZE, visitor)
}
}
Expand Down Expand Up @@ -464,10 +464,10 @@ impl PublicKey {
pub fn from_slice(data: &[u8]) -> Result<PublicKey, Error> {
match data.len() {
constants::PUBLIC_KEY_SIZE => PublicKey::from_byte_array_compressed(
&<[u8; constants::PUBLIC_KEY_SIZE]>::try_from(data).unwrap(),
<[u8; constants::PUBLIC_KEY_SIZE]>::try_from(data).unwrap(),
),
constants::UNCOMPRESSED_PUBLIC_KEY_SIZE => PublicKey::from_byte_array_uncompressed(
&<[u8; constants::UNCOMPRESSED_PUBLIC_KEY_SIZE]>::try_from(data).unwrap(),
<[u8; constants::UNCOMPRESSED_PUBLIC_KEY_SIZE]>::try_from(data).unwrap(),
),
_ => Err(InvalidPublicKey),
}
Expand All @@ -476,7 +476,7 @@ impl PublicKey {
/// Creates a public key from a serialized array in compressed format.
#[inline]
pub fn from_byte_array_compressed(
data: &[u8; constants::PUBLIC_KEY_SIZE],
data: [u8; constants::PUBLIC_KEY_SIZE],
) -> Result<PublicKey, Error> {
unsafe {
let mut pk = ffi::PublicKey::new();
Expand All @@ -497,7 +497,7 @@ impl PublicKey {
/// Creates a public key from a serialized array in uncompressed format.
#[inline]
pub fn from_byte_array_uncompressed(
data: &[u8; constants::UNCOMPRESSED_PUBLIC_KEY_SIZE],
data: [u8; constants::UNCOMPRESSED_PUBLIC_KEY_SIZE],
) -> Result<PublicKey, Error> {
unsafe {
let mut pk = ffi::PublicKey::new();
Expand Down Expand Up @@ -553,7 +553,7 @@ impl PublicKey {
};
buf[1..].clone_from_slice(&pk.serialize());

PublicKey::from_byte_array_compressed(&buf).expect("we know the buffer is valid")
PublicKey::from_byte_array_compressed(buf).expect("we know the buffer is valid")
}

#[inline]
Expand Down Expand Up @@ -790,10 +790,10 @@ impl<'de> serde::Deserialize<'de> for PublicKey {
"an ASCII hex string representing a public key",
))
} else {
let visitor = super::serde_util::Tuple33Visitor::new(
"33 bytes compressed public key",
PublicKey::from_slice,
);
let visitor =
super::serde_util::Tuple33Visitor::new("33 bytes compressed public key", |bytes| {
PublicKey::from_byte_array_compressed(bytes)
});
d.deserialize_tuple(constants::PUBLIC_KEY_SIZE, visitor)
}
}
Expand Down Expand Up @@ -859,16 +859,29 @@ impl Keypair {
/// # Errors
///
/// [`Error::InvalidSecretKey`] if the slice is not exactly 32 bytes long,
/// or if the encoded number exceeds the Secp256k1 field `p` value.
/// or if the encoded number is an invalid scalar.
#[deprecated(since = "TBD", note = "Use `from_seckey_byte_array` instead.")]
#[inline]
pub fn from_seckey_slice<C: Signing>(
secp: &Secp256k1<C>,
data: &[u8],
) -> Result<Keypair, Error> {
if data.is_empty() || data.len() != constants::SECRET_KEY_SIZE {
return Err(Error::InvalidSecretKey);
match <[u8; constants::SECRET_KEY_SIZE]>::try_from(data) {
Ok(data) => Self::from_seckey_byte_array(secp, data),
Err(_) => Err(Error::InvalidSecretKey),
}
}

/// Creates a [`Keypair`] directly from a secret key byte array.
///
/// # Errors
///
/// [`Error::InvalidSecretKey`] if the encoded number is an invalid scalar.
#[inline]
pub fn from_seckey_byte_array<C: Signing>(
secp: &Secp256k1<C>,
data: [u8; constants::SECRET_KEY_SIZE],
) -> Result<Keypair, Error> {
unsafe {
let mut kp = ffi::Keypair::new();
if ffi::secp256k1_keypair_create(secp.ctx.as_ptr(), &mut kp, data.as_c_ptr()) == 1 {
Expand All @@ -884,13 +897,12 @@ impl Keypair {
/// # Errors
///
/// [`Error::InvalidSecretKey`] if the string does not consist of exactly 64 hex characters,
/// or if the encoded number exceeds the Secp256k1 field `p` value.
/// or if the encoded number is an invalid scalar.
#[inline]
pub fn from_seckey_str<C: Signing>(secp: &Secp256k1<C>, s: &str) -> Result<Keypair, Error> {
let mut res = [0u8; constants::SECRET_KEY_SIZE];
match from_hex(s, &mut res) {
Ok(constants::SECRET_KEY_SIZE) =>
Keypair::from_seckey_slice(secp, &res[0..constants::SECRET_KEY_SIZE]),
Ok(constants::SECRET_KEY_SIZE) => Keypair::from_seckey_byte_array(secp, res),
_ => Err(Error::InvalidSecretKey),
}
}
Expand All @@ -900,7 +912,7 @@ impl Keypair {
/// # Errors
///
/// [`Error::InvalidSecretKey`] if the string does not consist of exactly 64 hex characters,
/// or if the encoded number exceeds the Secp256k1 field `p` value.
/// or if the encoded number is an invalid scalar.
#[inline]
#[cfg(feature = "global-context")]
pub fn from_seckey_str_global(s: &str) -> Result<Keypair, Error> {
Expand Down Expand Up @@ -1117,7 +1129,7 @@ impl<'de> serde::Deserialize<'de> for Keypair {
let ctx = Secp256k1::signing_only();

#[allow(clippy::needless_borrow)]
Keypair::from_seckey_slice(&ctx, data)
Keypair::from_seckey_byte_array(&ctx, data)
});
d.deserialize_tuple(constants::SECRET_KEY_SIZE, visitor)
}
Expand Down Expand Up @@ -1181,7 +1193,7 @@ impl str::FromStr for XOnlyPublicKey {
fn from_str(s: &str) -> Result<XOnlyPublicKey, Error> {
let mut res = [0u8; constants::SCHNORR_PUBLIC_KEY_SIZE];
match from_hex(s, &mut res) {
Ok(constants::SCHNORR_PUBLIC_KEY_SIZE) => XOnlyPublicKey::from_byte_array(&res),
Ok(constants::SCHNORR_PUBLIC_KEY_SIZE) => XOnlyPublicKey::from_byte_array(res),
_ => Err(Error::InvalidPublicKey),
}
}
Expand Down Expand Up @@ -1231,7 +1243,7 @@ impl XOnlyPublicKey {
#[inline]
pub fn from_slice(data: &[u8]) -> Result<XOnlyPublicKey, Error> {
match <[u8; constants::SCHNORR_PUBLIC_KEY_SIZE]>::try_from(data) {
Ok(data) => Self::from_byte_array(&data),
Ok(data) => Self::from_byte_array(data),
Err(_) => Err(InvalidPublicKey),
}
}
Expand All @@ -1244,7 +1256,7 @@ impl XOnlyPublicKey {
/// x coordinate.
#[inline]
pub fn from_byte_array(
data: &[u8; constants::SCHNORR_PUBLIC_KEY_SIZE],
data: [u8; constants::SCHNORR_PUBLIC_KEY_SIZE],
) -> Result<XOnlyPublicKey, Error> {
unsafe {
let mut pk = ffi::XOnlyPublicKey::new();
Expand Down Expand Up @@ -1597,7 +1609,7 @@ impl<'de> serde::Deserialize<'de> for XOnlyPublicKey {
} else {
let visitor = super::serde_util::Tuple32Visitor::new(
"raw 32 bytes schnorr public key",
XOnlyPublicKey::from_slice,
XOnlyPublicKey::from_byte_array,
);
d.deserialize_tuple(constants::SCHNORR_PUBLIC_KEY_SIZE, visitor)
}
Expand Down Expand Up @@ -1665,7 +1677,7 @@ mod test {
#[cfg(all(feature = "std", not(secp256k1_fuzz)))]
fn erased_keypair_is_valid() {
let s = Secp256k1::new();
let kp = Keypair::from_seckey_slice(&s, &[1u8; constants::SECRET_KEY_SIZE])
let kp = Keypair::from_seckey_byte_array(&s, [1u8; constants::SECRET_KEY_SIZE])
.expect("valid secret key");
let mut kp2 = kp;
kp2.non_secure_erase();
Expand Down Expand Up @@ -2272,7 +2284,7 @@ mod test {
];
static SK_STR: &str = "01010101010101010001020304050607ffff0000ffff00006363636363636363";

let sk = Keypair::from_seckey_slice(SECP256K1, &SK_BYTES).unwrap();
let sk = Keypair::from_seckey_byte_array(SECP256K1, SK_BYTES).unwrap();
#[rustfmt::skip]
assert_tokens(&sk.compact(), &[
Token::Tuple{ len: 32 },
Expand Down Expand Up @@ -2452,7 +2464,7 @@ mod test {

static PK_STR: &str = "18845781f631c48f1c9709e23092067d06837f30aa0cd0544ac887fe91ddd166";

let kp = Keypair::from_seckey_slice(crate::SECP256K1, &SK_BYTES).unwrap();
let kp = Keypair::from_seckey_byte_array(crate::SECP256K1, SK_BYTES).unwrap();
let (pk, _parity) = XOnlyPublicKey::from_keypair(&kp);

#[rustfmt::skip]
Expand Down
6 changes: 3 additions & 3 deletions src/schnorr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ mod tests {
0x63, 0x63, 0x63, 0x63,
];

let kp = Keypair::from_seckey_slice(&secp, &SK_BYTES).expect("sk");
let kp = Keypair::from_seckey_byte_array(&secp, SK_BYTES).expect("sk");

// In fuzzing mode secret->public key derivation is different, so
// hard-code the expected result.
Expand Down Expand Up @@ -473,7 +473,7 @@ mod tests {
let s = Secp256k1::new();

let msg = [1; 32];
let keypair = Keypair::from_seckey_slice(&s, &[2; 32]).unwrap();
let keypair = Keypair::from_seckey_byte_array(&s, [2; 32]).unwrap();
let aux = [3u8; 32];
let sig = s.sign_schnorr_with_aux_rand(&msg, &keypair, &aux);
static SIG_BYTES: [u8; constants::SCHNORR_SIGNATURE_SIZE] = [
Expand Down Expand Up @@ -706,7 +706,7 @@ mod tests {
} in vectors
{
if let (Some(secret_key), Some(aux_rand)) = (secret_key, aux_rand) {
let keypair = Keypair::from_seckey_slice(&secp, &secret_key).unwrap();
let keypair = Keypair::from_seckey_byte_array(&secp, secret_key).unwrap();
assert_eq!(keypair.x_only_public_key().0.serialize(), public_key);
let sig = secp.sign_schnorr_with_aux_rand(&message, &keypair, &aux_rand);
assert_eq!(sig.to_byte_array(), signature);
Expand Down
6 changes: 3 additions & 3 deletions src/serde_util.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ macro_rules! impl_tuple_visitor {

impl<F, T, E> $thing<F>
where
F: FnOnce(&[u8]) -> Result<T, E>,
F: FnOnce([u8; $len]) -> Result<T, E>,
E: fmt::Display,
{
pub fn new(expectation: &'static str, parse_fn: F) -> Self {
Expand All @@ -84,7 +84,7 @@ macro_rules! impl_tuple_visitor {

impl<'de, F, T, E> de::Visitor<'de> for $thing<F>
where
F: FnOnce(&[u8]) -> Result<T, E>,
F: FnOnce([u8; $len]) -> Result<T, E>,
E: fmt::Display,
{
type Value = T;
Expand All @@ -106,7 +106,7 @@ macro_rules! impl_tuple_visitor {
return Err(de::Error::invalid_length(i, &self));
}
}
(self.parse_fn)(&bytes).map_err(de::Error::custom)
(self.parse_fn)(bytes).map_err(de::Error::custom)
}
}
};
Expand Down
2 changes: 1 addition & 1 deletion tests/serde.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ fn bincode_public_key() {
#[cfg(feature = "global-context")]
fn bincode_keypair() {
let secp = Secp256k1::new();
let kp = Keypair::from_seckey_slice(&secp, &SK_BYTES).expect("failed to create keypair");
let kp = Keypair::from_seckey_byte_array(&secp, SK_BYTES).expect("failed to create keypair");
let ser = bincode::serialize(&kp).unwrap();

assert_eq!(ser, SK_BYTES);
Expand Down

0 comments on commit ede62d5

Please sign in to comment.