From 0b62f5e212e8d5d33d4753db9168d3817f2f802a Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 25 May 2022 20:17:06 +1000 Subject: [PATCH 1/3] Fix incorrect timelock docs We have mixed up relative/absolute on the trait methods for `from_after` and `from_older`. --- src/miniscript/types/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/miniscript/types/mod.rs b/src/miniscript/types/mod.rs index ac8b35998..b7a9a48e9 100644 --- a/src/miniscript/types/mod.rs +++ b/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) From e4eb285400c417cf9e7255b0e7911649afb201ee Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 25 May 2022 21:07:10 +1000 Subject: [PATCH 2/3] Add a minimal timelock module Currently if we mix up height/time absolute timelocks when filtering policies the result is incorrect. Add a `timelock` module that includes a single public function for checking if absolute timelocks are the same unit. Use the new function to fix a bug in policy filtering. --- src/lib.rs | 1 + src/policy/semantic.rs | 6 ++++-- src/timelock.rs | 21 +++++++++++++++++++++ 3 files changed, 26 insertions(+), 2 deletions(-) create mode 100644 src/timelock.rs diff --git a/src/lib.rs b/src/lib.rs index 5c061a07a..bc9715e86 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -104,6 +104,7 @@ pub mod interpreter; pub mod miniscript; pub mod policy; pub mod psbt; +pub mod timelock; mod util; diff --git a/src/policy/semantic.rs b/src/policy/semantic.rs index 4411aa899..5c04b476c 100644 --- a/src/policy/semantic.rs +++ b/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 Policy { pub fn at_height(mut self, time: u32) -> Policy { 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) diff --git a/src/timelock.rs b/src/timelock.rs new file mode 100644 index 000000000..a28e211e8 --- /dev/null +++ b/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 +} From 42e12168f4c4bcb6f521016930fa6a92289631fd Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 25 May 2022 20:52:39 +1000 Subject: [PATCH 3/3] Unit test mixed up absolute timelocks Currently if we mix up height/time absolute timelocks when filtering policies the result is incorrect. Add a bunch of assertions that verify the bug. --- src/policy/semantic.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/policy/semantic.rs b/src/policy/semantic.rs index 5c04b476c..122d2b26e 100644 --- a/src/policy/semantic.rs +++ b/src/policy/semantic.rs @@ -764,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]); @@ -772,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)); }