Merge #461: Cleanly separate experimental/insane miniscripts from sane miniscripts

6a06a11 Allow parsing expr fragments from string/script (sanket1729)
94d5eba Cleanup psbt translator APIs (sanket1729)
fff727c Remove Hash associated type from RawPkh. (sanket1729)
328d15f Rename the Semantic::translate_pkh to translate_pk (sanket1729)
b1d6477 Change Semantic KeyHash(Pk::Hash) to Key(Pk) (sanket1729)
372377a Remove PkTranslator trait (sanket1729)

Pull request description:

  Cleanly separates how we deal with insane and unspecified miniscripts in the library. Insane minsicripts are well-defined that could
  - It is unsafe(does not require a digital signature to spend it)
  - It contains a unspendable path because of either a) Resource limitations or b) Timelock Mixing
  - The script has malleable satisfaction
  - It has repeated public keys

  There are some unspecified features like PartialDescriptors that we use to allow parsing miniscripts from scripts or inferring descriptors. For this only includes
  - Parsing rawpkh fragments with `hash160`

  All of these can be individually enabled or disabled using the `ExtParams` builder.

  As a consequence of this, we removed the Miniscript::Hash associated type and change the Semantic policy to operate only on keys.

  Also fixes another issue in the usage of PkTranslator that made it impossible to implement the trait in downstream libraries

ACKs for top commit:
  apoelstra:
    ACK 6a06a11

Tree-SHA512: 200cbe8bd9a42f75db8f5c74c6e811d70210f2202352beb82326f6ada3b19ce683feab06419dac2f2c94c8fa5ea0f8ee1fa5f9c84a4ce2e8193b5a631eddd111

Author: apoelstra

examples/htlc.rs

@@ -42,13 +42,13 @@ fn main() {
     assert!(htlc_descriptor.sanity_check().is_ok());
     assert_eq!(
         format!("{}", htlc_descriptor),
-        "wsh(andor(pk(022222222222222222222222222222222222222222222222222222222222222222),sha256(1111111111111111111111111111111111111111111111111111111111111111),and_v(v:pkh(51814f108670aced2d77c1805ddd6634bc9d4731),older(4444))))#s0qq76ng"
+        "wsh(andor(pk(022222222222222222222222222222222222222222222222222222222222222222),sha256(1111111111111111111111111111111111111111111111111111111111111111),and_v(v:pkh(020202020202020202020202020202020202020202020202020202020202020202),older(4444))))#lfytrjen"
     );
 
     // Lift the descriptor into an abstract policy.
     assert_eq!(
         format!("{}", htlc_descriptor.lift().unwrap()),
-        "or(and(pkh(4377a5acd66dc5cb67148a24818d1e51fa183bd2),sha256(1111111111111111111111111111111111111111111111111111111111111111)),and(pkh(51814f108670aced2d77c1805ddd6634bc9d4731),older(4444)))"
+        "or(and(pk(022222222222222222222222222222222222222222222222222222222222222222),sha256(1111111111111111111111111111111111111111111111111111111111111111)),and(pk(020202020202020202020202020202020202020202020202020202020202020202),older(4444)))"
     );
 
     // Get the scriptPpubkey for this Wsh descriptor.

examples/taproot.rs

@@ -1,12 +1,11 @@
 use std::collections::HashMap;
 use std::str::FromStr;
 
-use bitcoin::hashes::{hash160, ripemd160, sha256};
 use bitcoin::util::address::WitnessVersion;
 use bitcoin::Network;
 use miniscript::descriptor::DescriptorType;
 use miniscript::policy::Concrete;
-use miniscript::{hash256, Descriptor, Miniscript, Tap, TranslatePk, Translator};
+use miniscript::{translate_hash_fail, Descriptor, Miniscript, Tap, TranslatePk, Translator};
 use secp256k1::{rand, KeyPair};
 
 // Refer to https://github.com/sanket1729/adv_btc_workshop/blob/master/workshop.md#creating-a-taproot-descriptor
@@ -21,25 +20,10 @@ impl Translator<String, bitcoin::XOnlyPublicKey, ()> for StrPkTranslator {
         self.pk_map.get(pk).copied().ok_or(())
     }
 
-    fn pkh(&mut self, _pkh: &String) -> Result<hash160::Hash, ()> {
-        unreachable!("Policy doesn't contain any pkh fragment");
-    }
-
-    fn sha256(&mut self, _sha256: &String) -> Result<sha256::Hash, ()> {
-        unreachable!("Policy does not contain any sha256 fragment");
-    }
-
-    fn hash256(&mut self, _sha256: &String) -> Result<hash256::Hash, ()> {
-        unreachable!("Policy does not contain any hash256 fragment");
-    }
-
-    fn ripemd160(&mut self, _ripemd160: &String) -> Result<ripemd160::Hash, ()> {
-        unreachable!("Policy does not contain any ripemd160 fragment");
-    }
-
-    fn hash160(&mut self, _hash160: &String) -> Result<hash160::Hash, ()> {
-        unreachable!("Policy does not contain any hash160 fragment");
-    }
+    // We don't need to implement these methods as we are not using them in the policy
+    // Fail if we encounter any hash fragments.
+    // See also translate_hash_clone! macro
+    translate_hash_fail!(String, bitcoin::XOnlyPublicKey, ());
 }
 
 fn main() {

src/descriptor/bare.rs

@@ -167,7 +167,6 @@ impl<Pk: MiniscriptKey> ForEachKey<Pk> for Bare<Pk> {
     fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, pred: F) -> bool
     where
         Pk: 'a,
-        Pk::RawPkHash: 'a,
     {
         self.ms.for_each_key(pred)
     }
@@ -294,7 +293,7 @@ impl<Pk: MiniscriptKey> fmt::Display for Pkh<Pk> {
 
 impl<Pk: MiniscriptKey> Liftable<Pk> for Pkh<Pk> {
     fn lift(&self) -> Result<semantic::Policy<Pk>, Error> {
-        Ok(semantic::Policy::KeyHash(self.pk.to_pubkeyhash()))
+        Ok(semantic::Policy::Key(self.pk.clone()))
     }
 }
 
@@ -329,7 +328,6 @@ impl<Pk: MiniscriptKey> ForEachKey<Pk> for Pkh<Pk> {
     fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, mut pred: F) -> bool
     where
         Pk: 'a,
-        Pk::RawPkHash: 'a,
     {
         pred(&self.pk)
     }

src/descriptor/key.rs

@@ -730,8 +730,6 @@ impl<K: InnerXKey> DescriptorXKey<K> {
 }
 
 impl MiniscriptKey for DescriptorPublicKey {
-    // This allows us to be able to derive public keys even for PkH s
-    type RawPkHash = Self;
     type Sha256 = sha256::Hash;
     type Hash256 = hash256::Hash;
     type Ripemd160 = ripemd160::Hash;
@@ -756,10 +754,6 @@ impl MiniscriptKey for DescriptorPublicKey {
             _ => false,
         }
     }
-
-    fn to_pubkeyhash(&self) -> Self {
-        self.clone()
-    }
 }
 
 impl DefiniteDescriptorKey {
@@ -818,8 +812,6 @@ impl fmt::Display for DefiniteDescriptorKey {
 }
 
 impl MiniscriptKey for DefiniteDescriptorKey {
-    // This allows us to be able to derive public keys even for PkH s
-    type RawPkHash = Self;
     type Sha256 = sha256::Hash;
     type Hash256 = hash256::Hash;
     type Ripemd160 = ripemd160::Hash;
@@ -832,10 +824,6 @@ impl MiniscriptKey for DefiniteDescriptorKey {
     fn is_x_only_key(&self) -> bool {
         self.0.is_x_only_key()
     }
-
-    fn to_pubkeyhash(&self) -> Self {
-        self.clone()
-    }
 }
 
 impl ToPublicKey for DefiniteDescriptorKey {
@@ -844,10 +832,6 @@ impl ToPublicKey for DefiniteDescriptorKey {
         self.0.derive_public_key(&secp).unwrap()
     }
 
-    fn hash_to_hash160(hash: &Self) -> hash160::Hash {
-        hash.to_public_key().to_pubkeyhash()
-    }
-
     fn to_sha256(hash: &sha256::Hash) -> sha256::Hash {
         *hash
     }

src/descriptor/mod.rs

@@ -37,8 +37,8 @@ use self::checksum::verify_checksum;
 use crate::miniscript::{Legacy, Miniscript, Segwitv0};
 use crate::prelude::*;
 use crate::{
-    expression, hash256, miniscript, BareCtx, Error, ForEachKey, MiniscriptKey, PkTranslator,
-    Satisfier, ToPublicKey, TranslatePk, Translator,
+    expression, hash256, miniscript, BareCtx, Error, ForEachKey, MiniscriptKey, Satisfier,
+    ToPublicKey, TranslatePk, Translator,
 };
 
 mod bare;
@@ -498,7 +498,6 @@ impl<Pk: MiniscriptKey> ForEachKey<Pk> for Descriptor<Pk> {
     fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, pred: F) -> bool
     where
         Pk: 'a,
-        Pk::RawPkHash: 'a,
     {
         match *self {
             Descriptor::Bare(ref bare) => bare.for_each_key(pred),
@@ -532,14 +531,12 @@ impl Descriptor<DescriptorPublicKey> {
     pub fn at_derivation_index(&self, index: u32) -> Descriptor<DefiniteDescriptorKey> {
         struct Derivator(u32);
 
-        impl PkTranslator<DescriptorPublicKey, DefiniteDescriptorKey, ()> for Derivator {
+        impl Translator<DescriptorPublicKey, DefiniteDescriptorKey, ()> for Derivator {
             fn pk(&mut self, pk: &DescriptorPublicKey) -> Result<DefiniteDescriptorKey, ()> {
                 Ok(pk.clone().at_derivation_index(self.0))
             }
 
-            fn pkh(&mut self, pkh: &DescriptorPublicKey) -> Result<DefiniteDescriptorKey, ()> {
-                Ok(pkh.clone().at_derivation_index(self.0))
-            }
+            translate_hash_clone!(DescriptorPublicKey, DescriptorPublicKey, ());
         }
         self.translate_pk(&mut Derivator(index))
             .expect("BIP 32 key index substitution cannot fail")
@@ -629,10 +626,6 @@ impl Descriptor<DescriptorPublicKey> {
                 parse_key(pk, &mut self.0, self.1)
             }
 
-            fn pkh(&mut self, pkh: &String) -> Result<DescriptorPublicKey, Error> {
-                parse_key(pkh, &mut self.0, self.1)
-            }
-
             fn sha256(&mut self, sha256: &String) -> Result<sha256::Hash, Error> {
                 let hash =
                     sha256::Hash::from_str(sha256).map_err(|e| Error::Unexpected(e.to_string()))?;
@@ -675,10 +668,6 @@ impl Descriptor<DescriptorPublicKey> {
                 key_to_string(pk, self.0)
             }
 
-            fn pkh(&mut self, pkh: &DescriptorPublicKey) -> Result<String, ()> {
-                key_to_string(pkh, self.0)
-            }
-
             fn sha256(&mut self, sha256: &sha256::Hash) -> Result<String, ()> {
                 Ok(sha256.to_string())
             }
@@ -766,7 +755,7 @@ impl Descriptor<DefiniteDescriptorKey> {
         struct Derivator<'a, C: secp256k1::Verification>(&'a secp256k1::Secp256k1<C>);
 
         impl<'a, C: secp256k1::Verification>
-            PkTranslator<DefiniteDescriptorKey, bitcoin::PublicKey, ConversionError>
+            Translator<DefiniteDescriptorKey, bitcoin::PublicKey, ConversionError>
             for Derivator<'a, C>
         {
             fn pk(
@@ -776,12 +765,7 @@ impl Descriptor<DefiniteDescriptorKey> {
                 pk.derive_public_key(&self.0)
             }
 
-            fn pkh(
-                &mut self,
-                pkh: &DefiniteDescriptorKey,
-            ) -> Result<bitcoin::hashes::hash160::Hash, ConversionError> {
-                Ok(pkh.derive_public_key(&self.0)?.to_pubkeyhash())
-            }
+            translate_hash_clone!(DefiniteDescriptorKey, bitcoin::PublicKey, ConversionError);
         }
 
         let derived = self.translate_pk(&mut Derivator(secp))?;
@@ -1340,8 +1324,8 @@ mod tests {
         assert_eq!(
             descriptor,
             format!(
-                "tr({},{{pk({}),{{pk({}),or_d(pk({}),pkh(516ca378e588a7ed71336147e2a72848b20aca1a))}}}})#xz8ny8ae",
-                p1, p2, p3, p4,
+                "tr({},{{pk({}),{{pk({}),or_d(pk({}),pkh({}))}}}})#tvu28c0s",
+                p1, p2, p3, p4, p5
             )
         )
     }

src/descriptor/segwitv0.rs

@@ -249,7 +249,6 @@ impl<Pk: MiniscriptKey> ForEachKey<Pk> for Wsh<Pk> {
     fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, pred: F) -> bool
     where
         Pk: 'a,
-        Pk::RawPkHash: 'a,
     {
         match self.inner {
             WshInner::SortedMulti(ref smv) => smv.for_each_key(pred),
@@ -407,7 +406,7 @@ impl<Pk: MiniscriptKey> fmt::Display for Wpkh<Pk> {
 
 impl<Pk: MiniscriptKey> Liftable<Pk> for Wpkh<Pk> {
     fn lift(&self) -> Result<semantic::Policy<Pk>, Error> {
-        Ok(semantic::Policy::KeyHash(self.pk.to_pubkeyhash()))
+        Ok(semantic::Policy::Key(self.pk.clone()))
     }
 }
 
@@ -442,7 +441,6 @@ impl<Pk: MiniscriptKey> ForEachKey<Pk> for Wpkh<Pk> {
     fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, mut pred: F) -> bool
     where
         Pk: 'a,
-        Pk::RawPkHash: 'a,
     {
         pred(&self.pk)
     }

src/descriptor/sh.rs

@@ -59,7 +59,7 @@ impl<Pk: MiniscriptKey> Liftable<Pk> for Sh<Pk> {
     fn lift(&self) -> Result<semantic::Policy<Pk>, Error> {
         match self.inner {
             ShInner::Wsh(ref wsh) => wsh.lift(),
-            ShInner::Wpkh(ref pk) => Ok(semantic::Policy::KeyHash(pk.as_inner().to_pubkeyhash())),
+            ShInner::Wpkh(ref pk) => Ok(semantic::Policy::Key(pk.as_inner().clone())),
             ShInner::SortedMulti(ref smv) => smv.lift(),
             ShInner::Ms(ref ms) => ms.lift(),
         }
@@ -380,7 +380,6 @@ impl<Pk: MiniscriptKey> ForEachKey<Pk> for Sh<Pk> {
     fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, pred: F) -> bool
     where
         Pk: 'a,
-        Pk::RawPkHash: 'a,
     {
         match self.inner {
             ShInner::Wsh(ref wsh) => wsh.for_each_key(pred),

src/descriptor/sortedmulti.rs

@@ -115,7 +115,6 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> ForEachKey<Pk> for SortedMultiVec<Pk
     fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, mut pred: F) -> bool
     where
         Pk: 'a,
-        Pk::RawPkHash: 'a,
     {
         self.pks.iter().all(|key| pred(key))
     }
@@ -216,9 +215,8 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> policy::Liftable<Pk> for SortedMulti
         let ret = policy::semantic::Policy::Threshold(
             self.k,
             self.pks
-                .clone()
-                .into_iter()
-                .map(|k| policy::semantic::Policy::KeyHash(k.to_pubkeyhash()))
+                .iter()
+                .map(|k| policy::semantic::Policy::Key(k.clone()))
                 .collect(),
         );
         Ok(ret)

src/descriptor/tr.rs

@@ -558,12 +558,9 @@ impl<Pk: MiniscriptKey> Liftable<Pk> for Tr<Pk> {
         match &self.tree {
             Some(root) => Ok(Policy::Threshold(
                 1,
-                vec![
-                    Policy::KeyHash(self.internal_key.to_pubkeyhash()),
-                    root.lift()?,
-                ],
+                vec![Policy::Key(self.internal_key.clone()), root.lift()?],
             )),
-            None => Ok(Policy::KeyHash(self.internal_key.to_pubkeyhash())),
+            None => Ok(Policy::Key(self.internal_key.clone())),
         }
     }
 }
@@ -572,7 +569,6 @@ impl<Pk: MiniscriptKey> ForEachKey<Pk> for Tr<Pk> {
     fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, mut pred: F) -> bool
     where
         Pk: 'a,
-        Pk::RawPkHash: 'a,
     {
         let script_keys_res = self
             .iter_scripts()