Use un/tweaked public key types

src/util/address.rs

@@ -527,11 +527,12 @@ impl Address {
         merkle_root: Option<TapBranchHash>,
         network: Network
     ) -> Address {
+        let (output_key, _parity) = internal_key.tap_tweak(secp, merkle_root);
         Address {
             network: network,
             payload: Payload::WitnessProgram {
                 version: WitnessVersion::V1,
-                program: internal_key.tap_tweak(secp, merkle_root).into_inner().serialize().to_vec()
+                program: output_key.into_inner().serialize().to_vec()
             }
         }
     }

src/util/schnorr.rs

@@ -26,6 +26,7 @@ use util::taproot::{TapBranchHash, TapTweakHash};
 pub type UntweakedPublicKey = PublicKey;
 
 /// Tweaked Schnorr public key
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct TweakedPublicKey(PublicKey);
 
 /// A trait for tweaking Schnorr public keys
@@ -38,26 +39,26 @@ pub trait TapTweak {
     ///  * H is the hash function
     ///  * c is the commitment data
     ///  * G is the generator point
-    fn tap_tweak<C: Verification>(self, secp: &Secp256k1<C>, merkle_root: Option<TapBranchHash>) -> TweakedPublicKey;
+    ///
+    /// # Returns
+    /// The tweaked key and its parity.
+    fn tap_tweak<C: Verification>(self, secp: &Secp256k1<C>, merkle_root: Option<TapBranchHash>) -> (TweakedPublicKey, bool);
 
-    /// Directly convert an UntweakedPublicKey to a TweakedPublicKey
+    /// Directly converts an [`UntweakedPublicKey`] to a [`TweakedPublicKey`]
     ///
     /// This method is dangerous and can lead to loss of funds if used incorrectly.
     /// Specifically, in multi-party protocols a peer can provide a value that allows them to steal.
     fn dangerous_assume_tweaked(self) -> TweakedPublicKey;
 }
 
 impl TapTweak for UntweakedPublicKey {
-    fn tap_tweak<C: Verification>(self, secp: &Secp256k1<C>, merkle_root: Option<TapBranchHash>) -> TweakedPublicKey {
-        // Compute the tweak
+    fn tap_tweak<C: Verification>(self, secp: &Secp256k1<C>, merkle_root: Option<TapBranchHash>) -> (TweakedPublicKey, bool) {
         let tweak_value = TapTweakHash::from_key_and_tweak(self, merkle_root).into_inner();
-
-        //Tweak the internal key by the tweak value
         let mut output_key = self.clone();
         let parity = output_key.tweak_add_assign(&secp, &tweak_value).expect("Tap tweak failed");
-        if self.tweak_add_check(&secp, &output_key, parity, tweak_value) {
-            return TweakedPublicKey(output_key);
-        } else { unreachable!("Tap tweak failed") }
+
+        debug_assert!(self.tweak_add_check(&secp, &output_key, parity, tweak_value));
+        (TweakedPublicKey(output_key), parity)
     }
 
     fn dangerous_assume_tweaked(self) -> TweakedPublicKey {
@@ -67,19 +68,20 @@ impl TapTweak for UntweakedPublicKey {
 
 
 impl TweakedPublicKey {
-    /// Create a new [TweakedPublicKey] from a [PublicKey]. No tweak is applied.
-    pub fn new(key: PublicKey) -> TweakedPublicKey {
+    /// Creates a new [`TweakedPublicKey`] from a [`PublicKey`]. No tweak is applied, consider
+    /// calling `tap_tweak` on an [`UntweakedPublicKey`] instead of using this constructor.
+    pub fn dangerous_assume_tweaked(key: PublicKey) -> TweakedPublicKey {
         TweakedPublicKey(key)
     }
 
-    /// Returns the underlying public key
+    /// Returns the underlying public key.
     pub fn into_inner(self) -> PublicKey {
         self.0
     }
 
-    /// Returns a reference to underlying public key
+    /// Returns a reference to underlying public key.
     pub fn as_inner(&self) -> &PublicKey {
         &self.0
     }
 
-}
+}

src/util/taproot.rs

@@ -25,7 +25,7 @@ use core::cmp::Reverse;
 use std::error;
 
 use hashes::{sha256, sha256t, Hash, HashEngine};
-use schnorr;
+use schnorr::{TweakedPublicKey, UntweakedPublicKey, TapTweak};
 use Script;
 
 use consensus::Encodable;
@@ -101,7 +101,7 @@ impl TapTweakHash {
     /// Create a new BIP341 [`TapTweakHash`] from key and tweak
     /// Produces H_taptweak(P||R) where P is internal key and R is the merkle root
     pub fn from_key_and_tweak(
-        internal_key: schnorr::PublicKey,
+        internal_key: UntweakedPublicKey,
         merkle_root: Option<TapBranchHash>,
     ) -> TapTweakHash {
         let mut eng = TapTweakHash::engine();
@@ -171,13 +171,13 @@ type ScriptMerkleProofMap = BTreeMap<(Script, LeafVersion), BTreeSet<TaprootMerk
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct TaprootSpendInfo {
     /// The BIP341 internal key.
-    internal_key: schnorr::PublicKey,
+    internal_key: UntweakedPublicKey,
     /// The Merkle root of the script tree (None if there are no scripts)
     merkle_root: Option<TapBranchHash>,
     /// The sign final output pubkey as per BIP 341
     output_key_parity: bool,
     /// The tweaked output key
-    output_key: schnorr::PublicKey,
+    output_key: TweakedPublicKey,
     /// Map from (script, leaf_version) to (sets of) [`TaprootMerkleBranch`].
     /// More than one control block for a given script is only possible if it
     /// appears in multiple branches of the tree. In all cases, keeping one should
@@ -204,7 +204,7 @@ impl TaprootSpendInfo {
     /// dealing with numbers close to 2^64.
     pub fn with_huffman_tree<C, I>(
         secp: &Secp256k1<C>,
-        internal_key: schnorr::PublicKey,
+        internal_key: UntweakedPublicKey,
         script_weights: I,
     ) -> Result<Self, TaprootBuilderError>
     where
@@ -250,20 +250,10 @@ impl TaprootSpendInfo {
     ///
     pub fn new_key_spend<C: secp256k1::Verification>(
         secp: &Secp256k1<C>,
-        internal_key: schnorr::PublicKey,
+        internal_key: UntweakedPublicKey,
         merkle_root: Option<TapBranchHash>,
     ) -> Self {
-        let tweak = TapTweakHash::from_key_and_tweak(internal_key, merkle_root);
-        let mut output_key = internal_key;
-        // # Panics:
-        //
-        // This would return Err if the merkle root hash is the negation of the secret
-        // key corresponding to the internal key.
-        // Because the tweak is derived as specified in BIP341 (hash output of a function),
-        // this is unlikely to occur (1/2^128) in real life usage, it is safe to unwrap this
-        let parity = output_key
-            .tweak_add_assign(&secp, &tweak)
-            .expect("TapTweakHash::from_key_and_tweak is broken");
+        let (output_key, parity) = internal_key.tap_tweak(secp, merkle_root);
         Self {
             internal_key: internal_key,
             merkle_root: merkle_root,
@@ -279,7 +269,7 @@ impl TaprootSpendInfo {
     }
 
     /// Obtain the internal key
-    pub fn internal_key(&self) -> schnorr::PublicKey {
+    pub fn internal_key(&self) -> UntweakedPublicKey {
         self.internal_key
     }
 
@@ -290,7 +280,7 @@ impl TaprootSpendInfo {
 
     /// Output key(the key used in script pubkey) from Spend data. See also
     /// [`TaprootSpendInfo::output_key_parity`]
-    pub fn output_key(&self) -> schnorr::PublicKey {
+    pub fn output_key(&self) -> TweakedPublicKey {
         self.output_key
     }
 
@@ -302,7 +292,7 @@ impl TaprootSpendInfo {
     // Internal function to compute [`TaprootSpendInfo`] from NodeInfo
     fn from_node_info<C: secp256k1::Verification>(
         secp: &Secp256k1<C>,
-        internal_key: schnorr::PublicKey,
+        internal_key: UntweakedPublicKey,
         node: NodeInfo,
     ) -> TaprootSpendInfo {
         // Create as if it is a key spend path with the given merkle root
@@ -430,7 +420,7 @@ impl TaprootBuilder {
     pub fn finalize<C: secp256k1::Verification>(
         mut self,
         secp: &Secp256k1<C>,
-        internal_key: schnorr::PublicKey,
+        internal_key: UntweakedPublicKey,
     ) -> Result<TaprootSpendInfo, TaprootBuilderError> {
         if self.branch.len() > 1 {
             return Err(TaprootBuilderError::IncompleteTree);
@@ -652,7 +642,7 @@ pub struct ControlBlock {
     /// The parity of the output key (NOT THE INTERNAL KEY WHICH IS ALWAYS XONLY)
     pub output_key_parity: bool,
     /// The internal key
-    pub internal_key: schnorr::PublicKey,
+    pub internal_key: UntweakedPublicKey,
     /// The merkle proof of a script associated with this leaf
     pub merkle_branch: TaprootMerkleBranch,
 }
@@ -674,7 +664,7 @@ impl ControlBlock {
         }
         let output_key_parity = (sl[0] & 1) == 1;
         let leaf_version = LeafVersion::from_u8(sl[0] & TAPROOT_LEAF_MASK)?;
-        let internal_key = schnorr::PublicKey::from_slice(&sl[1..TAPROOT_CONTROL_BASE_SIZE])
+        let internal_key = UntweakedPublicKey::from_slice(&sl[1..TAPROOT_CONTROL_BASE_SIZE])
             .map_err(TaprootError::InvalidInternalKey)?;
         let merkle_branch = TaprootMerkleBranch::from_slice(&sl[TAPROOT_CONTROL_BASE_SIZE..])?;
         Ok(ControlBlock {
@@ -719,7 +709,7 @@ impl ControlBlock {
     pub fn verify_taproot_commitment<C: secp256k1::Verification>(
         &self,
         secp: &Secp256k1<C>,
-        output_key: &schnorr::PublicKey,
+        output_key: &TweakedPublicKey,
         script: &Script,
     ) -> bool {
         // compute the script hash
@@ -743,7 +733,7 @@ impl ControlBlock {
         let tweak = TapTweakHash::from_key_and_tweak(self.internal_key, Some(curr_hash));
         self.internal_key.tweak_add_check(
             secp,
-            output_key,
+            output_key.as_inner(),
             self.output_key_parity,
             tweak.into_inner(),
         )
@@ -900,6 +890,7 @@ mod test {
     use hashes::{sha256, Hash, HashEngine};
     use secp256k1::VerifyOnly;
     use core::str::FromStr;
+    use schnorr;
 
     fn tag_engine(tag_name: &str) -> sha256::HashEngine {
         let mut engine = sha256::Hash::engine();
@@ -984,6 +975,7 @@ mod test {
 
     fn _verify_tap_commitments(secp: &Secp256k1<VerifyOnly>, out_spk_hex: &str, script_hex : &str, control_block_hex: &str) {
         let out_pk = schnorr::PublicKey::from_str(&out_spk_hex[4..]).unwrap();
+        let out_pk = TweakedPublicKey::dangerous_assume_tweaked(out_pk);
         let script = Script::from_hex(script_hex).unwrap();
         let control_block = ControlBlock::from_slice(&Vec::<u8>::from_hex(control_block_hex).unwrap()).unwrap();
         assert_eq!(control_block_hex, control_block.serialize().to_hex());
@@ -1025,7 +1017,7 @@ mod test {
     #[test]
     fn build_huffman_tree() {
         let secp = Secp256k1::verification_only();
-        let internal_key = schnorr::PublicKey::from_str("93c7378d96518a75448821c4f7c8f4bae7ce60f804d03d1f0628dd5dd0f5de51").unwrap();
+        let internal_key = UntweakedPublicKey::from_str("93c7378d96518a75448821c4f7c8f4bae7ce60f804d03d1f0628dd5dd0f5de51").unwrap();
 
         let script_weights = vec![
             (10, Script::from_hex("51").unwrap()), // semantics of script don't matter for this test
@@ -1075,7 +1067,7 @@ mod test {
     #[test]
     fn taptree_builder() {
         let secp = Secp256k1::verification_only();
-        let internal_key = schnorr::PublicKey::from_str("93c7378d96518a75448821c4f7c8f4bae7ce60f804d03d1f0628dd5dd0f5de51").unwrap();
+        let internal_key = UntweakedPublicKey::from_str("93c7378d96518a75448821c4f7c8f4bae7ce60f804d03d1f0628dd5dd0f5de51").unwrap();
 
         let builder = TaprootBuilder::new();
         // Create a tree as shown below