Fix bug in absolute timelock usage

src/lib.rs

@@ -104,6 +104,7 @@ pub mod interpreter;
 pub mod miniscript;
 pub mod policy;
 pub mod psbt;
+pub mod timelock;
 
 mod util;
 

src/miniscript/types/mod.rs

@@ -302,13 +302,13 @@ pub trait Property: Sized {
     /// Type property of a timelock
     fn from_time(t: u32) -> Self;
 
-    /// Type property of a relative timelock. Default implementation simply
+    /// Type property of an absolute timelock. Default implementation simply
     /// passes through to `from_time`
     fn from_after(t: u32) -> Self {
         Self::from_time(t)
     }
 
-    /// Type property of an absolute timelock. Default implementation simply
+    /// Type property of a relative timelock. Default implementation simply
     /// passes through to `from_time`
     fn from_older(t: u32) -> Self {
         Self::from_time(t)

src/policy/semantic.rs

@@ -22,7 +22,7 @@ use bitcoin::hashes::{hash160, ripemd160, sha256, sha256d};
 
 use super::concrete::PolicyError;
 use super::ENTAILMENT_MAX_TERMINALS;
-use crate::{errstr, expression, Error, ForEach, ForEachKey, MiniscriptKey};
+use crate::{errstr, expression, timelock, Error, ForEach, ForEachKey, MiniscriptKey};
 
 /// Abstract policy which corresponds to the semantics of a Miniscript
 /// and which allows complex forms of analysis, e.g. filtering and
@@ -543,7 +543,9 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
     pub fn at_height(mut self, time: u32) -> Policy<Pk> {
         self = match self {
             Policy::After(t) => {
-                if t > time {
+                if !timelock::absolute_timelocks_are_same_unit(t, time) {
+                    Policy::Unsatisfiable
+                } else if t > time {
                     Policy::Unsatisfiable
                 } else {
                     Policy::After(t)
@@ -762,6 +764,7 @@ mod tests {
         assert_eq!(policy.n_keys(), 0);
         assert_eq!(policy.minimum_n_keys(), Some(0));
 
+        // Block height 1000.
         let policy = StringPolicy::from_str("after(1000)").unwrap();
         assert_eq!(policy, Policy::After(1000));
         assert_eq!(policy.absolute_timelocks(), vec![1000]);
@@ -770,6 +773,26 @@ mod tests {
         assert_eq!(policy.clone().at_height(999), Policy::Unsatisfiable);
         assert_eq!(policy.clone().at_height(1000), policy.clone());
         assert_eq!(policy.clone().at_height(10000), policy.clone());
+        // Pass a UNIX timestamp to at_height while policy uses a block height.
+        assert_eq!(policy.clone().at_height(500_000_001), Policy::Unsatisfiable);
+        assert_eq!(policy.n_keys(), 0);
+        assert_eq!(policy.minimum_n_keys(), Some(0));
+
+        // UNIX timestamp of 10 seconds after the epoch.
+        let policy = StringPolicy::from_str("after(500000010)").unwrap();
+        assert_eq!(policy, Policy::After(500_000_010));
+        assert_eq!(policy.absolute_timelocks(), vec![500_000_010]);
+        assert_eq!(policy.relative_timelocks(), vec![]);
+        // Pass a block height to at_height while policy uses a UNIX timestapm.
+        assert_eq!(policy.clone().at_height(0), Policy::Unsatisfiable);
+        assert_eq!(policy.clone().at_height(999), Policy::Unsatisfiable);
+        assert_eq!(policy.clone().at_height(1000), Policy::Unsatisfiable);
+        assert_eq!(policy.clone().at_height(10000), Policy::Unsatisfiable);
+        // And now pass a UNIX timestamp to at_height while policy also uses a timestamp.
+        assert_eq!(policy.clone().at_height(500_000_000), Policy::Unsatisfiable);
+        assert_eq!(policy.clone().at_height(500_000_001), Policy::Unsatisfiable);
+        assert_eq!(policy.clone().at_height(500_000_010), policy.clone());
+        assert_eq!(policy.clone().at_height(500_000_012), policy.clone());
         assert_eq!(policy.n_keys(), 0);
         assert_eq!(policy.minimum_n_keys(), Some(0));
     }

src/timelock.rs

@@ -0,0 +1,21 @@
+//! Various functions for manipulating Bitcoin timelocks.
+
+use crate::miniscript::limits::LOCKTIME_THRESHOLD;
+
+/// Returns true if `a` and `b` are the same unit i.e., both are block heights or both are UNIX
+/// timestamps. `a` and `b` are nLockTime values.
+pub fn absolute_timelocks_are_same_unit(a: u32, b: u32) -> bool {
+    n_lock_time_is_block_height(a) == n_lock_time_is_block_height(b)
+}
+
+// https://github.com/bitcoin/bitcoin/blob/9ccaee1d5e2e4b79b0a7c29aadb41b97e4741332/src/script/script.h#L39
+
+/// Returns true if nLockTime `n` is to be interpreted as a block height.
+pub fn n_lock_time_is_block_height(n: u32) -> bool {
+    n < LOCKTIME_THRESHOLD
+}
+
+/// Returns true if nLockTime `n` is to be interpreted as a UNIX timestamp.
+pub fn n_lock_time_is_timestamp(n: u32) -> bool {
+    n >= LOCKTIME_THRESHOLD
+}