Update lift trait

Added height and timelocks types for detecting if there
is atleast one spending path that contains a combination
of those.
Updated the lifting trait so that it can fail

examples/htlc.rs

@@ -39,7 +39,7 @@ fn main() {
     );
 
     assert_eq!(
-        format!("{}", htlc_descriptor.lift()),
+        format!("{}", htlc_descriptor.lift().unwrap()),
         "or(and(pkh(4377a5acd66dc5cb67148a24818d1e51fa183bd2),and(pkh(4377a5acd66dc5cb67148a24818d1e51fa183bd2),older(4444))),sha256(1111111111111111111111111111111111111111111111111111111111111111))"
     );
 

src/miniscript/mod.rs

@@ -659,7 +659,7 @@ mod tests {
             keys[4].to_string(),
         );
 
-        let mut abs = miniscript.lift();
+        let mut abs = miniscript.lift().unwrap();
         assert_eq!(abs.n_keys(), 5);
         assert_eq!(abs.minimum_n_keys(), 2);
         abs = abs.at_age(10000);

src/policy/compiler.rs

@@ -1191,10 +1191,28 @@ mod tests {
         let policy = DummyPolicy::from_str(s).expect("parse");
         let miniscript: Miniscript<DummyKey, Segwitv0> = policy.compile()?;
 
-        assert_eq!(policy.lift().sorted(), miniscript.lift().sorted());
+        assert_eq!(
+            policy.lift().unwrap().sorted(),
+            miniscript.lift().unwrap().sorted()
+        );
         Ok(())
     }
 
+    #[test]
+    fn compile_timelocks() {
+        // artificially create a policy that is problematic and try to compile
+        let pol: DummyPolicy = Concrete::And(vec![
+            Concrete::Key(DummyKey),
+            Concrete::And(vec![Concrete::After(9), Concrete::After(1000_000_000)]),
+        ]);
+        assert!(pol.compile::<Segwitv0>().is_err());
+
+        // This should compile
+        let pol: DummyPolicy =
+            DummyPolicy::from_str("and(pk(),or(and(after(9),pk()),and(after(1000000000),pk())))")
+                .unwrap();
+        assert!(pol.compile::<Segwitv0>().is_ok());
+    }
     #[test]
     fn compile_basic() {
         assert!(policy_compile_lift_check("pk()").is_ok());
@@ -1234,7 +1252,10 @@ mod tests {
             best_t(&mut HashMap::new(), &policy, 1.0, None).unwrap();
 
         assert_eq!(compilation.cost_1d(1.0, None), 88.0 + 74.109375);
-        assert_eq!(policy.lift().sorted(), compilation.ms.lift().sorted());
+        assert_eq!(
+            policy.lift().unwrap().sorted(),
+            compilation.ms.lift().unwrap().sorted()
+        );
 
         let policy = SPolicy::from_str(
                 "and(and(and(or(127@thresh(2,pk(),pk(),thresh(2,or(127@pk(),1@pk()),after(100),or(and(pk(),after(200)),and(pk(),sha256(66687aadf862bd776c8fc18b8e9f8e20089714856ee233b3902a591d0d5f2925))),pk())),1@pk()),sha256(66687aadf862bd776c8fc18b8e9f8e20089714856ee233b3902a591d0d5f2925)),or(127@pk(),1@after(300))),or(127@after(400),pk()))"
@@ -1243,7 +1264,10 @@ mod tests {
             best_t(&mut HashMap::new(), &policy, 1.0, None).unwrap();
 
         assert_eq!(compilation.cost_1d(1.0, None), 437.0 + 299.4003295898438);
-        assert_eq!(policy.lift().sorted(), compilation.ms.lift().sorted());
+        assert_eq!(
+            policy.lift().unwrap().sorted(),
+            compilation.ms.lift().unwrap().sorted()
+        );
     }
 
     #[test]
@@ -1313,7 +1337,7 @@ mod tests {
 
         assert_eq!(ms, ms_comp_res);
 
-        let mut abs = policy.lift();
+        let mut abs = policy.lift().unwrap();
         assert_eq!(abs.n_keys(), 8);
         assert_eq!(abs.minimum_n_keys(), 2);
         abs = abs.at_age(10000);

src/policy/concrete.rs

@@ -21,6 +21,7 @@ use std::{error, fmt, str};
 
 use errstr;
 use expression::{self, FromTree};
+use miniscript::types::extra_props::HEIGHT_TIME_THRESHOLD;
 #[cfg(feature = "compiler")]
 use miniscript::ScriptContext;
 #[cfg(feature = "compiler")]
@@ -30,7 +31,6 @@ use policy::compiler::CompilerError;
 #[cfg(feature = "compiler")]
 use Miniscript;
 use {Error, MiniscriptKey};
-
 /// Concrete policy which corresponds directly to a Miniscript structure,
 /// and whose disjunctions are annotated with satisfaction probabilities
 /// to assist the compiler
@@ -72,6 +72,9 @@ pub enum PolicyError {
     ZeroTime,
     /// `after` fragment can only have ` n < 2^31`
     TimeTooFar,
+    /// lifting error: Cannot lift policies that have
+    /// a combination of height and timelocks.
+    HeightTimeLockCombination,
 }
 
 impl error::Error for PolicyError {
@@ -97,6 +100,9 @@ impl fmt::Display for PolicyError {
                 f.write_str("Relative/Absolute time must be less than 2^31; n < 2^31")
             }
             PolicyError::ZeroTime => f.write_str("Time must be greater than 0; n > 0"),
+            PolicyError::HeightTimeLockCombination => {
+                f.write_str("Cannot lift polcies that have a heightlock and timelock combination")
+            }
         }
     }
 }
@@ -150,9 +156,112 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
         }
     }
 
+    /// Checks whether the given concrete policy contains a combination of
+    /// timelocks and heightlocks.
+    /// Returns an error if there is atleast one satisfaction that contains
+    /// a combination of hieghtlock and timelock.
+    pub fn check_timelocks(&self) -> Result<(), PolicyError> {
+        let timelocks = self.check_timelocks_helper();
+        if timelocks[4] {
+            Err(PolicyError::HeightTimeLockCombination)
+        } else {
+            Ok(())
+        }
+    }
+
+    // Checks whether the given concrete policy contains a combination of
+    // timelocks and heightlocks
+    // Returns a tuple of five elements:
+    // timelocks[0]: satisfaction contains csv heightlock
+    // timelocks[1]: satisfaction contains csv timelock
+    // timelocks[2]: satisfaction contains cltv heightlock
+    // timelocks[3]: satisfaction contains cltv timelock
+    // timelocks[4]: satisfaction contains an invalid combination
+    fn check_timelocks_helper(&self) -> [bool; 5] {
+        // timelocks[csv_h, csv_t, cltv_h, cltv_t, combination]
+        match *self {
+            Policy::Key(_)
+            | Policy::Sha256(_)
+            | Policy::Hash256(_)
+            | Policy::Ripemd160(_)
+            | Policy::Hash160(_) => [false; 5],
+            Policy::After(t) => [
+                false,
+                false,
+                t < HEIGHT_TIME_THRESHOLD,
+                t >= HEIGHT_TIME_THRESHOLD,
+                false,
+            ],
+            Policy::Older(t) => [
+                t < HEIGHT_TIME_THRESHOLD,
+                t >= HEIGHT_TIME_THRESHOLD,
+                false,
+                false,
+                false,
+            ],
+            Policy::Threshold(k, ref subs) => {
+                let mut timelocks = [false; 5];
+                // k >= 2 implies it is possible to have timelocks and heightlocks
+                // merged together
+                // timelocks[0] and timelocks[1] can be directly
+                // combined together instead of iterating through all the pairs:
+                // consider the two cases:
+                // 1) Only one subfragment contains timelock and hieghtlock: In this case,
+                // timelocks[4] would already by `true`. and other combinations would
+                // not add any effect.
+                // 2) hieghtlocks and timelocks are distributed across two branches:
+                // and we check if any of the existing branches has correponding lock
+                // that cannot be combined
+                // If k == 1, then this is no combination possible.
+                for sub in subs {
+                    let sub_timelocks = sub.check_timelocks_helper();
+                    if k >= 2 {
+                        timelocks[4] |= (timelocks[0] && sub_timelocks[1])
+                            || (timelocks[1] && sub_timelocks[0])
+                            || (timelocks[3] && sub_timelocks[2])
+                            || (timelocks[2] && sub_timelocks[3]);
+                    }
+                    for i in 0..5 {
+                        timelocks[i] |= sub_timelocks[i];
+                    }
+                }
+                timelocks
+            }
+            Policy::And(ref subs) => {
+                let mut timelocks = [false; 5];
+                for sub in subs {
+                    let sub_timelocks = sub.check_timelocks_helper();
+                    // Check if by adding the new `sub` we create a
+                    // new combination that could be problematic
+                    timelocks[4] |= (timelocks[0] && sub_timelocks[1])
+                        || (timelocks[1] && sub_timelocks[0])
+                        || (timelocks[3] && sub_timelocks[2])
+                        || (timelocks[2] && sub_timelocks[3]);
+                    for i in 0..5 {
+                        timelocks[i] |= sub_timelocks[i];
+                    }
+                }
+                timelocks
+            }
+            Policy::Or(ref subs) => {
+                let mut timelocks = [false; 5];
+                for &(ref _prob, ref sub) in subs {
+                    let sub_timelocks = sub.check_timelocks_helper();
+                    for i in 0..5 {
+                        timelocks[i] |= sub_timelocks[i];
+                    }
+                }
+                timelocks
+            }
+        }
+    }
+
     /// This returns whether the given policy is valid or not. It maybe possible that the policy
     /// contains Non-two argument `and`, `or` or a `0` arg thresh.
+    /// Validity condition also checks whether there is a possible satisfaction
+    /// combination of timelocks and heightlocks
     pub fn is_valid(&self) -> Result<(), PolicyError> {
+        self.check_timelocks()?;
         match *self {
             Policy::And(ref subs) => {
                 if subs.len() != 2 {
@@ -344,7 +453,9 @@ where
         }
 
         let tree = expression::Tree::from_str(s)?;
-        FromTree::from_tree(&tree)
+        let policy: Policy<Pk> = FromTree::from_tree(&tree)?;
+        policy.check_timelocks()?;
+        Ok(policy)
     }
 }