Several locktime improvements

src/descriptor/mod.rs

@@ -512,7 +512,7 @@ impl Descriptor<DefiniteDescriptorKey> {
                 descriptor: self,
                 template: stack,
                 absolute_timelock: satisfaction.absolute_timelock.map(Into::into),
-                relative_timelock: satisfaction.relative_timelock,
+                relative_timelock: satisfaction.relative_timelock.map(Into::into),
             })
         } else {
             Err(self)
@@ -540,7 +540,8 @@ impl Descriptor<DefiniteDescriptorKey> {
                 descriptor: self,
                 template: stack,
                 absolute_timelock: satisfaction.absolute_timelock.map(Into::into),
-                relative_timelock: satisfaction.relative_timelock,
+                // unwrap to be removed in a later commit
+                relative_timelock: satisfaction.relative_timelock.map(Into::into),
             })
         } else {
             Err(self)

src/interpreter/error.rs

@@ -9,7 +9,7 @@ use bitcoin::hashes::hash160;
 use bitcoin::hex::DisplayHex;
 #[cfg(not(test))] // https://github.com/rust-lang/rust/issues/121684
 use bitcoin::secp256k1;
-use bitcoin::taproot;
+use bitcoin::{absolute, relative, taproot};
 
 use super::BitcoinKey;
 use crate::prelude::*;
@@ -18,9 +18,9 @@ use crate::prelude::*;
 #[derive(Debug)]
 pub enum Error {
     /// Could not satisfy, absolute locktime not met
-    AbsoluteLocktimeNotMet(u32),
+    AbsoluteLockTimeNotMet(absolute::LockTime),
     /// Could not satisfy, lock time values are different units
-    AbsoluteLocktimeComparisonInvalid(u32, u32),
+    AbsoluteLockTimeComparisonInvalid(absolute::LockTime, absolute::LockTime),
     /// Cannot Infer a taproot descriptor
     /// Key spends cannot infer the internal key of the descriptor
     /// Inferring script spends is possible, but is hidden nodes are currently
@@ -85,7 +85,9 @@ pub enum Error {
     /// Parse Error while parsing a `stack::Element::Push` as a XOnlyPublicKey (32 bytes)
     XOnlyPublicKeyParseError,
     /// Could not satisfy, relative locktime not met
-    RelativeLocktimeNotMet(u32),
+    RelativeLockTimeNotMet(relative::LockTime),
+    /// Could not satisfy, the sequence number on the tx input had the disable flag set.
+    RelativeLockTimeDisabled(relative::LockTime),
     /// Forward-secp related errors
     Secp(secp256k1::Error),
     /// Miniscript requires the entire top level script to be satisfied.
@@ -116,10 +118,10 @@ pub enum Error {
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match *self {
-            Error::AbsoluteLocktimeNotMet(n) => {
+            Error::AbsoluteLockTimeNotMet(n) => {
                 write!(f, "required absolute locktime CLTV of {} blocks, not met", n)
             }
-            Error::AbsoluteLocktimeComparisonInvalid(n, lock_time) => write!(
+            Error::AbsoluteLockTimeComparisonInvalid(n, lock_time) => write!(
                 f,
                 "could not satisfy, lock time values are different units n: {} lock_time: {}",
                 n, lock_time
@@ -159,9 +161,12 @@ impl fmt::Display for Error {
             Error::PkHashVerifyFail(ref hash) => write!(f, "Pubkey Hash check failed {}", hash),
             Error::PubkeyParseError => f.write_str("could not parse pubkey"),
             Error::XOnlyPublicKeyParseError => f.write_str("could not parse x-only pubkey"),
-            Error::RelativeLocktimeNotMet(n) => {
+            Error::RelativeLockTimeNotMet(n) => {
                 write!(f, "required relative locktime CSV of {} blocks, not met", n)
             }
+            Error::RelativeLockTimeDisabled(n) => {
+                write!(f, "required relative locktime CSV of {} blocks, but tx sequence number has disable-flag set", n)
+            }
             Error::ScriptSatisfactionError => f.write_str("Top level script must be satisfied"),
             Error::Secp(ref e) => fmt::Display::fmt(e, f),
             Error::SchnorrSig(ref s) => write!(f, "Schnorr sig error: {}", s),
@@ -188,8 +193,8 @@ impl error::Error for Error {
         use self::Error::*;
 
         match self {
-            AbsoluteLocktimeNotMet(_)
-            | AbsoluteLocktimeComparisonInvalid(_, _)
+            AbsoluteLockTimeNotMet(_)
+            | AbsoluteLockTimeComparisonInvalid(_, _)
             | CannotInferTrDescriptors
             | ControlBlockVerificationError
             | CouldNotEvaluate
@@ -212,7 +217,8 @@ impl error::Error for Error {
             | XOnlyPublicKeyParseError
             | PkEvaluationError(_)
             | PkHashVerifyFail(_)
-            | RelativeLocktimeNotMet(_)
+            | RelativeLockTimeNotMet(_)
+            | RelativeLockTimeDisabled(_)
             | ScriptSatisfactionError
             | TapAnnexUnsupported
             | UncompressedPubkey

src/interpreter/mod.rs

@@ -12,7 +12,7 @@ use core::fmt;
 use core::str::FromStr;
 
 use bitcoin::hashes::{hash160, ripemd160, sha256, Hash};
-use bitcoin::{absolute, secp256k1, sighash, taproot, Sequence, TxOut, Witness};
+use bitcoin::{absolute, relative, secp256k1, sighash, taproot, Sequence, TxOut, Witness};
 
 use crate::miniscript::context::{NoChecks, SigType};
 use crate::miniscript::ScriptContext;
@@ -35,7 +35,7 @@ pub struct Interpreter<'txin> {
     /// For non-Taproot spends, the scriptCode; for Taproot script-spends, this
     /// is the leaf script; for key-spends it is `None`.
     script_code: Option<bitcoin::ScriptBuf>,
-    age: Sequence,
+    sequence: Sequence,
     lock_time: absolute::LockTime,
 }
 
@@ -136,11 +136,11 @@ impl<'txin> Interpreter<'txin> {
         spk: &bitcoin::ScriptBuf,
         script_sig: &'txin bitcoin::Script,
         witness: &'txin Witness,
-        age: Sequence,                 // CSV, relative lock time.
+        sequence: Sequence,            // CSV, relative lock time.
         lock_time: absolute::LockTime, // CLTV, absolute lock time.
     ) -> Result<Self, Error> {
         let (inner, stack, script_code) = inner::from_txdata(spk, script_sig, witness)?;
-        Ok(Interpreter { inner, stack, script_code, age, lock_time })
+        Ok(Interpreter { inner, stack, script_code, sequence, lock_time })
     }
 
     /// Same as [`Interpreter::iter`], but allows for a custom verification function.
@@ -165,7 +165,7 @@ impl<'txin> Interpreter<'txin> {
             // Cloning the references to elements of stack should be fine as it allows
             // call interpreter.iter() without mutating interpreter
             stack: self.stack.clone(),
-            age: self.age,
+            sequence: self.sequence,
             lock_time: self.lock_time,
             has_errored: false,
             sig_type: self.sig_type(),
@@ -468,7 +468,7 @@ pub enum SatisfiedConstraint {
     ///Relative Timelock for CSV.
     RelativeTimelock {
         /// The value of RelativeTimelock
-        n: Sequence,
+        n: relative::LockTime,
     },
     ///Absolute Timelock for CLTV.
     AbsoluteTimelock {
@@ -508,7 +508,7 @@ pub struct Iter<'intp, 'txin: 'intp> {
     public_key: Option<&'intp BitcoinKey>,
     state: Vec<NodeEvaluationState<'intp>>,
     stack: Stack<'txin>,
-    age: Sequence,
+    sequence: Sequence,
     lock_time: absolute::LockTime,
     has_errored: bool,
     sig_type: SigType,
@@ -608,7 +608,7 @@ where
                 Terminal::Older(ref n) => {
                     debug_assert_eq!(node_state.n_evaluated, 0);
                     debug_assert_eq!(node_state.n_satisfied, 0);
-                    let res = self.stack.evaluate_older(n, self.age);
+                    let res = self.stack.evaluate_older(&(*n).into(), self.sequence);
                     if res.is_some() {
                         return res;
                     }
@@ -1110,7 +1110,7 @@ mod tests {
                 stack,
                 public_key: None,
                 state: vec![NodeEvaluationState { node: ms, n_evaluated: 0, n_satisfied: 0 }],
-                age: Sequence::from_height(1002),
+                sequence: Sequence::from_height(1002),
                 lock_time: absolute::LockTime::from_height(1002).unwrap(),
                 has_errored: false,
                 sig_type: SigType::Ecdsa,
@@ -1182,7 +1182,9 @@ mod tests {
         let older_satisfied: Result<Vec<SatisfiedConstraint>, Error> = constraints.collect();
         assert_eq!(
             older_satisfied.unwrap(),
-            vec![SatisfiedConstraint::RelativeTimelock { n: Sequence::from_height(1000) }]
+            vec![SatisfiedConstraint::RelativeTimelock {
+                n: crate::RelLockTime::from_height(1000).into()
+            }]
         );
 
         //Check Sha256

src/interpreter/stack.rs

@@ -5,7 +5,7 @@
 
 use bitcoin::blockdata::{opcodes, script};
 use bitcoin::hashes::{hash160, ripemd160, sha256, Hash};
-use bitcoin::{absolute, Sequence};
+use bitcoin::{absolute, relative, Sequence};
 
 use super::error::PkEvalErrInner;
 use super::{verify_sersig, BitcoinKey, Error, HashLockType, KeySigPair, SatisfiedConstraint};
@@ -212,19 +212,14 @@ impl<'txin> Stack<'txin> {
         let is_satisfied = match (*n, lock_time) {
             (Blocks(n), Blocks(lock_time)) => n <= lock_time,
             (Seconds(n), Seconds(lock_time)) => n <= lock_time,
-            _ => {
-                return Some(Err(Error::AbsoluteLocktimeComparisonInvalid(
-                    n.to_consensus_u32(),
-                    lock_time.to_consensus_u32(),
-                )))
-            }
+            _ => return Some(Err(Error::AbsoluteLockTimeComparisonInvalid(*n, lock_time))),
         };
 
         if is_satisfied {
             self.push(Element::Satisfied);
             Some(Ok(SatisfiedConstraint::AbsoluteTimelock { n: *n }))
         } else {
-            Some(Err(Error::AbsoluteLocktimeNotMet(n.to_consensus_u32())))
+            Some(Err(Error::AbsoluteLockTimeNotMet(*n)))
         }
     }
 
@@ -236,14 +231,19 @@ impl<'txin> Stack<'txin> {
     /// booleans
     pub(super) fn evaluate_older(
         &mut self,
-        n: &Sequence,
-        age: Sequence,
+        n: &relative::LockTime,
+        sequence: Sequence,
     ) -> Option<Result<SatisfiedConstraint, Error>> {
-        if age >= *n {
-            self.push(Element::Satisfied);
-            Some(Ok(SatisfiedConstraint::RelativeTimelock { n: *n }))
+        if let Some(tx_locktime) = sequence.to_relative_lock_time() {
+            if n.is_implied_by(tx_locktime) {
+                self.push(Element::Satisfied);
+                Some(Ok(SatisfiedConstraint::RelativeTimelock { n: *n }))
+            } else {
+                Some(Err(Error::RelativeLockTimeNotMet(*n)))
+            }
         } else {
-            Some(Err(Error::RelativeLocktimeNotMet(n.to_consensus_u32())))
+            // BIP 112: if the tx locktime has the disable flag set, fail CSV.
+            Some(Err(Error::RelativeLockTimeDisabled(*n)))
         }
     }
 

src/lib.rs

@@ -87,6 +87,7 @@
 #![deny(missing_docs)]
 // Clippy lints that we have disabled
 #![allow(clippy::iter_kv_map)] // https://github.com/rust-lang/rust-clippy/issues/11752
+#![allow(clippy::manual_range_contains)] // I hate this lint -asp
 
 #[cfg(target_pointer_width = "16")]
 compile_error!(
@@ -125,19 +126,19 @@ pub mod iter;
 pub mod miniscript;
 pub mod plan;
 pub mod policy;
+mod primitives;
 pub mod psbt;
 
 #[cfg(test)]
 mod test_utils;
 mod util;
 
-use core::{cmp, fmt, hash, str};
+use core::{fmt, hash, str};
 #[cfg(feature = "std")]
 use std::error;
 
 use bitcoin::hashes::{hash160, ripemd160, sha256, Hash};
 use bitcoin::hex::DisplayHex;
-use bitcoin::locktime::absolute;
 use bitcoin::{script, Opcode};
 
 pub use crate::blanket_traits::FromStrKey;
@@ -149,6 +150,8 @@ pub use crate::miniscript::decode::Terminal;
 pub use crate::miniscript::satisfy::{Preimage32, Satisfier};
 pub use crate::miniscript::{hash256, Miniscript};
 use crate::prelude::*;
+pub use crate::primitives::absolute_locktime::{AbsLockTime, AbsLockTimeError};
+pub use crate::primitives::relative_locktime::{RelLockTime, RelLockTimeError};
 
 /// Public key trait which can be converted to Hash type
 pub trait MiniscriptKey: Clone + Eq + Ord + fmt::Debug + fmt::Display + hash::Hash {
@@ -501,6 +504,10 @@ pub enum Error {
     /// At least two BIP389 key expressions in the descriptor contain tuples of
     /// derivation indexes of different lengths.
     MultipathDescLenMismatch,
+    /// Invalid absolute locktime
+    AbsoluteLockTime(AbsLockTimeError),
+    /// Invalid absolute locktime
+    RelativeLockTime(RelLockTimeError),
 }
 
 // https://github.com/sipa/miniscript/pull/5 for discussion on this number
@@ -576,6 +583,8 @@ impl fmt::Display for Error {
             Error::TrNoScriptCode => write!(f, "No script code for Tr descriptors"),
             Error::TrNoExplicitScript => write!(f, "No script code for Tr descriptors"),
             Error::MultipathDescLenMismatch => write!(f, "At least two BIP389 key expressions in the descriptor contain tuples of derivation indexes of different lengths"),
+            Error::AbsoluteLockTime(ref e) => e.fmt(f),
+            Error::RelativeLockTime(ref e) => e.fmt(f),
         }
     }
 }
@@ -628,6 +637,8 @@ impl error::Error for Error {
             ContextError(e) => Some(e),
             AnalysisError(e) => Some(e),
             PubKeyCtxError(e, _) => Some(e),
+            AbsoluteLockTime(e) => Some(e),
+            RelativeLockTime(e) => Some(e),
         }
     }
 }
@@ -711,51 +722,6 @@ fn hex_script(s: &str) -> bitcoin::ScriptBuf {
     bitcoin::ScriptBuf::from(v)
 }
 
-/// An absolute locktime that implements `Ord`.
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
-pub struct AbsLockTime(absolute::LockTime);
-
-impl AbsLockTime {
-    /// Constructs an `AbsLockTime` from an nLockTime value or the argument to OP_CHEKCLOCKTIMEVERIFY.
-    pub fn from_consensus(n: u32) -> Self { Self(absolute::LockTime::from_consensus(n)) }
-
-    /// Returns the inner `u32` value. This is the value used when creating this `LockTime`
-    /// i.e., `n OP_CHECKLOCKTIMEVERIFY` or nLockTime.
-    ///
-    /// This calls through to `absolute::LockTime::to_consensus_u32()` and the same usage warnings
-    /// apply.
-    pub fn to_consensus_u32(self) -> u32 { self.0.to_consensus_u32() }
-
-    /// Returns the inner `u32` value.
-    ///
-    /// Equivalent to `AbsLockTime::to_consensus_u32()`.
-    pub fn to_u32(self) -> u32 { self.to_consensus_u32() }
-}
-
-impl From<absolute::LockTime> for AbsLockTime {
-    fn from(lock_time: absolute::LockTime) -> Self { Self(lock_time) }
-}
-
-impl From<AbsLockTime> for absolute::LockTime {
-    fn from(lock_time: AbsLockTime) -> absolute::LockTime { lock_time.0 }
-}
-
-impl cmp::PartialOrd for AbsLockTime {
-    fn partial_cmp(&self, other: &Self) -> Option<cmp::Ordering> { Some(self.cmp(other)) }
-}
-
-impl cmp::Ord for AbsLockTime {
-    fn cmp(&self, other: &Self) -> cmp::Ordering {
-        let this = self.0.to_consensus_u32();
-        let that = other.0.to_consensus_u32();
-        this.cmp(&that)
-    }
-}
-
-impl fmt::Display for AbsLockTime {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt(&self.0, f) }
-}
-
 #[cfg(test)]
 mod tests {
     use core::str::FromStr;