From c612130864ef8043ea952cf5879d256511cfbf27 Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Thu, 24 Mar 2022 13:23:12 +1100 Subject: [PATCH 1/3] Borrow secret key `SecretKey` implements `Copy` and it is fine to take owneship of it; we have multiple methods called `from_secret_key` and they all borrow the secret key parameter. Favour consistency over perfection. Borrow secret key parameter as is done in other `from_secret_key` methods. --- src/key.rs | 6 +++--- src/secret.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/key.rs b/src/key.rs index 82ffef9..9b1a84b 100644 --- a/src/key.rs +++ b/src/key.rs @@ -674,7 +674,7 @@ impl Ord for PublicKey { /// /// let secp = Secp256k1::new(); /// let (secret_key, public_key) = secp.generate_keypair(&mut rand::thread_rng()); -/// let key_pair = KeyPair::from_secret_key(&secp, secret_key); +/// let key_pair = KeyPair::from_secret_key(&secp, &secret_key); /// # } /// ``` /// [`Deserialize`]: serde::Deserialize @@ -706,7 +706,7 @@ impl KeyPair { #[inline] pub fn from_secret_key( secp: &Secp256k1, - sk: SecretKey, + sk: &SecretKey, ) -> KeyPair { unsafe { let mut kp = ffi::KeyPair::new(); @@ -1426,7 +1426,7 @@ pub mod serde_keypair { Ok(KeyPair::from_secret_key( &::SECP256K1, - secret_key, + &secret_key, )) } } diff --git a/src/secret.rs b/src/secret.rs index 493070f..7ee08ac 100644 --- a/src/secret.rs +++ b/src/secret.rs @@ -160,7 +160,7 @@ impl KeyPair { /// /// let secp = Secp256k1::new(); /// let key = ONE_KEY; - /// let key = KeyPair::from_secret_key(&secp, key); + /// let key = KeyPair::from_secret_key(&secp, &key); /// // Here we explicitly display the secret value: /// assert_eq!( /// "0000000000000000000000000000000000000000000000000000000000000001", From b4c7fa0d4e0f679ce3afa9272cd8bf40350b4467 Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Thu, 24 Mar 2022 14:30:57 +1100 Subject: [PATCH 2/3] Let the compiler work out int size We have two places in the code where we pass a mutable parity integer to ffi code. At one callsite we tell the compiler explicitly what type it is (`::secp256k1_sys::types::c_int`) and at the other call site we let the compiler figure out the type. Is one way better than the other? I don't know. But letting the compiler figure it out seems to make the code easier to read. --- src/key.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/key.rs b/src/key.rs index 9b1a84b..79b76d5 100644 --- a/src/key.rs +++ b/src/key.rs @@ -1111,6 +1111,7 @@ impl XOnlyPublicKey { return Err(Error::InvalidTweak); } + let mut pk_parity = 0; unsafe { let mut pubkey = ffi::PublicKey::new(); let mut err = ffi::secp256k1_xonly_pubkey_tweak_add( @@ -1123,18 +1124,17 @@ impl XOnlyPublicKey { return Err(Error::InvalidTweak); } - let mut parity: ::secp256k1_sys::types::c_int = 0; err = ffi::secp256k1_xonly_pubkey_from_pubkey( secp.ctx, &mut self.0, - &mut parity, + &mut pk_parity, &pubkey, ); if err == 0 { return Err(Error::InvalidPublicKey); } - Parity::from_i32(parity).map_err(Into::into) + Parity::from_i32(pk_parity).map_err(Into::into) } } From f08276adfc82d3d703d575a9a5f3e15af4cf40b1 Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Thu, 24 Mar 2022 14:53:40 +1100 Subject: [PATCH 3/3] Add convenience methods for keys We have a bunch of `from_` methods for converting between key types. To improve the API and make it more ergonomic to use we can add methods that do the same but can be called on the initial key instead of on the resulting key's type. E.g. once applied the following are equivalent: - `let pk = PublicKey::from_keypair(kp)` - `let pk = kp.public_key()` Do this for `SecretKey`, `PublicKey`, `KeyPair`, and `XOnlyKeyPair`. --- src/key.rs | 253 +++++++++++++++++++++++++++++++++++++++++++++++-- src/schnorr.rs | 13 +-- 2 files changed, 251 insertions(+), 15 deletions(-) diff --git a/src/key.rs b/src/key.rs index 79b76d5..061bfea 100644 --- a/src/key.rs +++ b/src/key.rs @@ -292,6 +292,31 @@ impl SecretKey { pub fn sign_ecdsa(&self, msg: Message) -> ecdsa::Signature { SECP256K1.sign_ecdsa(&msg, self) } + + /// Returns the [`KeyPair`] for this [`SecretKey`]. + /// + /// This is equivalent to using [`KeyPair::from_secret_key`]. + #[inline] + pub fn keypair(&self, secp: &Secp256k1) -> KeyPair { + KeyPair::from_secret_key(secp, self) + } + + /// Returns the [`PublicKey`] for this [`SecretKey`]. + /// + /// This is equivalent to using [`PublicKey::from_secret_key`]. + #[inline] + pub fn public_key(&self, secp: &Secp256k1) -> PublicKey { + PublicKey::from_secret_key(secp, self) + } + + /// Returns the [`XOnlyPublicKey`] (and it's [`Parity`]) for this [`SecretKey`]. + /// + /// This is equivalent to `XOnlyPublicKey::from_keypair(self.keypair(secp))`. + #[inline] + pub fn x_only_public_key(&self, secp: &Secp256k1) -> (XOnlyPublicKey, Parity) { + let kp = self.keypair(secp); + XOnlyPublicKey::from_keypair(&kp) + } } #[cfg(feature = "serde")] @@ -418,6 +443,20 @@ impl PublicKey { } } + /// Creates a [`PublicKey`] using the key material from `pk` combined with the `parity`. + pub fn from_x_only_public_key(pk: XOnlyPublicKey, parity: Parity) -> PublicKey { + let mut buf = [0u8; 33]; + + // First byte of a compressed key should be `0x02 AND parity`. + buf[0] = match parity { + Parity::Even => 0x02, + Parity::Odd => 0x03, + }; + buf[1..].clone_from_slice(&pk.serialize()); + + PublicKey::from_slice(&buf).expect("we know the buffer is valid") + } + #[inline] /// Serializes the key as a byte-encoded pair of values. In compressed form the y-coordinate is /// represented by only a single bit, as x determines it up to one bit. @@ -589,6 +628,25 @@ impl PublicKey { } } } + + /// Returns the [`XOnlyPublicKey`] (and it's [`Parity`]) for this [`PublicKey`]. + #[inline] + pub fn x_only_public_key(&self) -> (XOnlyPublicKey, Parity) { + let mut pk_parity = 0; + unsafe { + let mut xonly_pk = ffi::XOnlyPublicKey::new(); + let ret = ffi::secp256k1_xonly_pubkey_from_pubkey( + ffi::secp256k1_context_no_precomp, + &mut xonly_pk, + &mut pk_parity, + self.as_ptr(), + ); + debug_assert_eq!(ret, 1); + let parity = Parity::from_i32(pk_parity).expect("should not panic, pk_parity is 0 or 1"); + + (XOnlyPublicKey(xonly_pk), parity) + } + } } impl CPtr for PublicKey { @@ -866,9 +924,27 @@ impl KeyPair { } } - /// Gets the [XOnlyPublicKey] for this [KeyPair]. + /// Returns the [`SecretKey`] for this [`KeyPair`]. + /// + /// This is equivalent to using [`SecretKey::from_keypair`]. #[inline] - pub fn public_key(&self) -> XOnlyPublicKey { + pub fn secret_key(&self) -> SecretKey { + SecretKey::from_keypair(self) + } + + /// Returns the [`PublicKey`] for this [`KeyPair`]. + /// + /// This is equivalent to using [`PublicKey::from_keypair`]. + #[inline] + pub fn public_key(&self) -> PublicKey { + PublicKey::from_keypair(self) + } + + /// Returns the [`XOnlyPublicKey`] (and it's [`Parity`]) for this [`KeyPair`]. + /// + /// This is equivalent to using [`XOnlyPublicKey::from_keypair`]. + #[inline] + pub fn x_only_public_key(&self) -> (XOnlyPublicKey, Parity) { XOnlyPublicKey::from_keypair(self) } @@ -1014,9 +1090,9 @@ impl XOnlyPublicKey { &mut self.0 } - /// Creates a new Schnorr public key from a Schnorr key pair. + /// Returns the [`XOnlyPublicKey`] (and it's [`Parity`]) for `keypair`. #[inline] - pub fn from_keypair(keypair: &KeyPair) -> XOnlyPublicKey { + pub fn from_keypair(keypair: &KeyPair) -> (XOnlyPublicKey, Parity) { let mut pk_parity = 0; unsafe { let mut xonly_pk = ffi::XOnlyPublicKey::new(); @@ -1027,7 +1103,9 @@ impl XOnlyPublicKey { keypair.as_ptr(), ); debug_assert_eq!(ret, 1); - XOnlyPublicKey(xonly_pk) + let parity = Parity::from_i32(pk_parity).expect("should not panic, pk_parity is 0 or 1"); + + (XOnlyPublicKey(xonly_pk), parity) } } @@ -1098,7 +1176,7 @@ impl XOnlyPublicKey { /// thread_rng().fill_bytes(&mut tweak); /// /// let mut key_pair = KeyPair::new(&secp, &mut thread_rng()); - /// let mut public_key = key_pair.public_key(); + /// let (mut public_key, _parity) = key_pair.x_only_public_key(); /// public_key.tweak_add_assign(&secp, &tweak).expect("Improbable to fail with a randomly generated tweak"); /// # } /// ``` @@ -1163,7 +1241,7 @@ impl XOnlyPublicKey { /// thread_rng().fill_bytes(&mut tweak); /// /// let mut key_pair = KeyPair::new(&secp, &mut thread_rng()); - /// let mut public_key = key_pair.public_key(); + /// let (mut public_key, _) = key_pair.x_only_public_key(); /// let original = public_key; /// let parity = public_key.tweak_add_assign(&secp, &tweak).expect("Improbable to fail with a randomly generated tweak"); /// assert!(original.tweak_add_check(&secp, &public_key, parity, tweak)); @@ -1189,6 +1267,14 @@ impl XOnlyPublicKey { err == 1 } } + + /// Returns the [`PublicKey`] for this [`XOnlyPublicKey`]. + /// + /// This is equivalent to using [`PublicKey::from_xonly_and_parity(self, parity)`]. + #[inline] + pub fn public_key(&self, parity: Parity) -> PublicKey { + PublicKey::from_x_only_public_key(*self, parity) + } } /// Represents the parity passed between FFI function calls. @@ -1978,12 +2064,15 @@ mod test { thread_rng().fill_bytes(&mut tweak); let mut kp = KeyPair::new(&s, &mut thread_rng()); - let mut pk = kp.public_key(); + let (mut pk, _parity) = kp.x_only_public_key(); let orig_pk = pk; kp.tweak_add_assign(&s, &tweak).expect("Tweak error"); let parity = pk.tweak_add_assign(&s, &tweak).expect("Tweak error"); - assert_eq!(XOnlyPublicKey::from_keypair(&kp), pk); + + let (back, _) = XOnlyPublicKey::from_keypair(&kp); + + assert_eq!(back, pk); assert!(orig_pk.tweak_add_check(&s, &pk, parity, tweak)); } } @@ -2052,4 +2141,150 @@ mod test { assert_tokens(&sk.readable(), &[Token::Str(SK_STR)]); assert_tokens(&sk.readable(), &[Token::String(SK_STR)]); } + + #[cfg(all(not(fuzzing), any(feature = "alloc", feature = "std")))] + fn keys() -> (SecretKey, PublicKey, KeyPair, XOnlyPublicKey) { + let secp = Secp256k1::new(); + + static SK_BYTES: [u8; 32] = [ + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, + 0xff, 0xff, 0x00, 0x00, 0xff, 0xff, 0x00, 0x00, + 0x63, 0x63, 0x63, 0x63, 0x63, 0x63, 0x63, 0x63, + ]; + + static PK_BYTES: [u8; 32] = [ + 0x18, 0x84, 0x57, 0x81, 0xf6, 0x31, 0xc4, 0x8f, + 0x1c, 0x97, 0x09, 0xe2, 0x30, 0x92, 0x06, 0x7d, + 0x06, 0x83, 0x7f, 0x30, 0xaa, 0x0c, 0xd0, 0x54, + 0x4a, 0xc8, 0x87, 0xfe, 0x91, 0xdd, 0xd1, 0x66 + ]; + + let mut pk_bytes = [0u8; 33]; + pk_bytes[0] = 0x02; // Use positive Y co-ordinate. + pk_bytes[1..].clone_from_slice(&PK_BYTES); + + let sk = SecretKey::from_slice(&SK_BYTES).expect("failed to parse sk bytes"); + let pk = PublicKey::from_slice(&pk_bytes).expect("failed to create pk from iterator"); + let kp = KeyPair::from_secret_key(&secp, &sk); + let xonly = XOnlyPublicKey::from_slice(&PK_BYTES).expect("failed to get xonly from slice"); + + (sk, pk, kp, xonly) + } + + #[test] + #[cfg(all(not(fuzzing), any(feature = "alloc", feature = "std")))] + fn convert_public_key_to_xonly_public_key() { + let (_sk, pk, _kp, want) = keys(); + let (got, parity) = pk.x_only_public_key(); + + assert_eq!(parity, Parity::Even); + assert_eq!(got, want) + } + + #[test] + #[cfg(all(not(fuzzing), any(feature = "alloc", feature = "std")))] + fn convert_secret_key_to_public_key() { + let secp = Secp256k1::new(); + + let (sk, want, _kp, _xonly) = keys(); + let got = sk.public_key(&secp); + + assert_eq!(got, want) + } + + #[test] + #[cfg(all(not(fuzzing), any(feature = "alloc", feature = "std")))] + fn convert_secret_key_to_x_only_public_key() { + let secp = Secp256k1::new(); + + let (sk, _pk, _kp, want) = keys(); + let (got, parity) = sk.x_only_public_key(&secp); + + assert_eq!(parity, Parity::Even); + assert_eq!(got, want) + } + + #[test] + #[cfg(all(not(fuzzing), any(feature = "alloc", feature = "std")))] + fn convert_keypair_to_public_key() { + let (_sk, want, kp, _xonly) = keys(); + let got = kp.public_key(); + + assert_eq!(got, want) + } + + #[test] + #[cfg(all(not(fuzzing), any(feature = "alloc", feature = "std")))] + fn convert_keypair_to_x_only_public_key() { + let (_sk, _pk, kp, want) = keys(); + let (got, parity) = kp.x_only_public_key(); + + assert_eq!(parity, Parity::Even); + assert_eq!(got, want) + } + + // SecretKey -> KeyPair -> SecretKey + #[test] + #[cfg(all(not(fuzzing), any(feature = "alloc", feature = "std")))] + fn roundtrip_secret_key_via_keypair() { + let secp = Secp256k1::new(); + let (sk, _pk, _kp, _xonly) = keys(); + + let kp = sk.keypair(&secp); + let back = kp.secret_key(); + + assert_eq!(back, sk) + } + + // KeyPair -> SecretKey -> KeyPair + #[test] + #[cfg(all(not(fuzzing), any(feature = "alloc", feature = "std")))] + fn roundtrip_keypair_via_secret_key() { + let secp = Secp256k1::new(); + let (_sk, _pk, kp, _xonly) = keys(); + + let sk = kp.secret_key(); + let back = sk.keypair(&secp); + + assert_eq!(back, kp) + } + + // XOnlyPublicKey -> PublicKey -> XOnlyPublicKey + #[test] + #[cfg(all(not(fuzzing), any(feature = "alloc", feature = "std")))] + fn roundtrip_x_only_public_key_via_public_key() { + let (_sk, _pk, _kp, xonly) = keys(); + + let pk = xonly.public_key(Parity::Even); + let (back, parity) = pk.x_only_public_key(); + + assert_eq!(parity, Parity::Even); + assert_eq!(back, xonly) + } + + // PublicKey -> XOnlyPublicKey -> PublicKey + #[test] + #[cfg(all(not(fuzzing), any(feature = "alloc", feature = "std")))] + fn roundtrip_public_key_via_x_only_public_key() { + let (_sk, pk, _kp, _xonly) = keys(); + + let (xonly, parity) = pk.x_only_public_key(); + let back = xonly.public_key(parity); + + assert_eq!(back, pk) + } + + #[test] + fn public_key_from_x_only_public_key_and_odd_parity() { + let s = "18845781f631c48f1c9709e23092067d06837f30aa0cd0544ac887fe91ddd166"; + let mut want = String::from("03"); + want.push_str(s); + + let xonly = XOnlyPublicKey::from_str(s).expect("failed to parse xonly pubkey string"); + let pk = xonly.public_key(Parity::Odd); + let got = format!("{}", pk); + + assert_eq!(got, want) + } } diff --git a/src/schnorr.rs b/src/schnorr.rs index 58683cb..523cf67 100644 --- a/src/schnorr.rs +++ b/src/schnorr.rs @@ -268,7 +268,7 @@ impl Secp256k1 { rng: &mut R, ) -> (KeyPair, XOnlyPublicKey) { let sk = KeyPair::new(self, rng); - let pubkey = XOnlyPublicKey::from_keypair(&sk); + let (pubkey, _parity) = XOnlyPublicKey::from_keypair(&sk); (sk, pubkey) } } @@ -344,7 +344,7 @@ mod tests { let mut rng = thread_rng(); let kp = KeyPair::new(&secp, &mut rng); - let pk = kp.public_key(); + let (pk, _parity) = kp.x_only_public_key(); let mut msg = [0u8; 32]; @@ -414,7 +414,7 @@ mod tests { fn test_pubkey_serialize_roundtrip() { let secp = Secp256k1::new(); let kp = KeyPair::new(&secp, &mut thread_rng()); - let pk = kp.public_key(); + let (pk, _parity) = kp.x_only_public_key(); let ser = pk.serialize(); let pubkey2 = XOnlyPublicKey::from_slice(&ser).unwrap(); @@ -431,7 +431,7 @@ mod tests { assert_eq!(SecretKey::from_str(sk_str).unwrap(), sk); let pk = ::key::PublicKey::from_keypair(&keypair); assert_eq!(::key::PublicKey::from_secret_key(&secp, &sk), pk); - let xpk = keypair.public_key(); + let (xpk, _parity) = keypair.x_only_public_key(); assert_eq!(XOnlyPublicKey::from(pk), xpk); } @@ -478,7 +478,8 @@ mod tests { // In fuzzing mode secret->public key derivation is different, so // hard-code the expected result. - kp.public_key() + let (pk, _parity) = kp.x_only_public_key(); + pk }; #[cfg(fuzzing)] let pk = XOnlyPublicKey::from_slice(&[0x18, 0x84, 0x57, 0x81, 0xf6, 0x31, 0xc4, 0x8f, 0x1c, 0x97, 0x09, 0xe2, 0x30, 0x92, 0x06, 0x7d, 0x06, 0x83, 0x7f, 0x30, 0xaa, 0x0c, 0xd0, 0x54, 0x4a, 0xc8, 0x87, 0xfe, 0x91, 0xdd, 0xd1, 0x66]).expect("pk"); @@ -544,7 +545,7 @@ mod tests { let secp = Secp256k1::new(); let kp = KeyPair::new(&secp, &mut DumbRng(0)); - let pk = kp.public_key(); + let (pk, _parity) = kp.x_only_public_key(); assert_eq!( &pk.serialize()[..], &[