Use the new relative::LockTime module

tcharding · tcharding · commit f8aa6590f0b1 · 2022-08-16T13:56:47.000+10:00
Implement `check_older` using the new `relative::LockTime` API.

This code seems upside down, I'm not sure I have the naming correct,
i.e., I don't know which (`self` or arg) is doing the satisfying and
which is satisfied?

I believe the `&lt;=` are going the correct direction, its the naming and
which sequence value to call the 'lock' that I'm struggling with.
diff --git a/Cargo.toml b/Cargo.toml
@@ -22,15 +22,15 @@ use-serde = ["serde", "bitcoin/serde"]
 rand = ["bitcoin/rand"]
 
 [dependencies]
-bitcoin = { version = "0.29.0", default-features = false }
+bitcoin = { path = "/home/tobin/build/github.com/tcharding/rust-bitcoin", default-features = false }
 serde = { version = "1.0", optional = true }
 hashbrown = { version = "0.11", optional = true }
 
 [dev-dependencies]
 bitcoind = { git = "https://github.com/tcharding/bitcoind", branch = "dev-build", features=["22_0"] }
 actual-rand = { package = "rand", version = "0.8.4"}
 secp256k1 = {version = "0.24.0", features = ["rand-std"]}
-bitcoin = { version = "0.29.0", features = ["rand"] }
+bitcoin = { path = "/home/tobin/build/github.com/tcharding/rust-bitcoin", features = ["rand"] }
 
 [[example]]
 name = "htlc"
diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs
@@ -25,7 +25,7 @@ use core::str::FromStr;
 use bitcoin::blockdata::witness::Witness;
 use bitcoin::hashes::{hash160, ripemd160, sha256};
 use bitcoin::util::{sighash, taproot};
-use bitcoin::{self, secp256k1, LockTime, Sequence, TxOut};
+use bitcoin::{self, secp256k1, absolute, Sequence, TxOut};
 
 use crate::miniscript::context::NoChecks;
 use crate::miniscript::ScriptContext;
@@ -49,7 +49,7 @@ pub struct Interpreter<'txin> {
     /// is the leaf script; for key-spends it is `None`.
     script_code: Option<bitcoin::Script>,
     age: Sequence,
-    lock_time: LockTime,
+    lock_time: absolute::LockTime,
 }
 
 // A type representing functions for checking signatures that accept both
@@ -174,7 +174,7 @@ impl<'txin> Interpreter<'txin> {
         script_sig: &'txin bitcoin::Script,
         witness: &'txin Witness,
         age: Sequence,       // CSV, relative lock time.
-        lock_time: LockTime, // CLTV, absolute 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 {
@@ -496,7 +496,7 @@ pub enum SatisfiedConstraint {
     ///Absolute Timelock for CLTV.
     AbsoluteTimelock {
         /// The value of Absolute timelock
-        n: LockTime,
+        n: absolute::LockTime,
     },
 }
 
@@ -532,7 +532,7 @@ pub struct Iter<'intp, 'txin: 'intp> {
     state: Vec<NodeEvaluationState<'intp>>,
     stack: Stack<'txin>,
     age: Sequence,
-    lock_time: LockTime,
+    lock_time: absolute::LockTime,
     has_errored: bool,
 }
 
@@ -1146,7 +1146,7 @@ mod tests {
                     n_satisfied: 0,
                 }],
                 age: Sequence::from_height(1002),
-                lock_time: LockTime::from_height(1002).unwrap(),
+                lock_time: absolute::LockTime::from_height(1002).unwrap(),
                 has_errored: false,
             }
         }
@@ -1210,7 +1210,7 @@ mod tests {
         assert_eq!(
             after_satisfied.unwrap(),
             vec![SatisfiedConstraint::AbsoluteTimelock {
-                n: LockTime::from_height(1000).unwrap()
+                n: absolute::LockTime::from_height(1000).unwrap()
             }]
         );
 
diff --git a/src/interpreter/stack.rs b/src/interpreter/stack.rs
@@ -17,7 +17,7 @@
 use bitcoin;
 use bitcoin::blockdata::{opcodes, script};
 use bitcoin::hashes::{hash160, ripemd160, sha256, Hash};
-use bitcoin::{LockTime, Sequence};
+use bitcoin::{absolute, Sequence};
 
 use super::error::PkEvalErrInner;
 use super::{
@@ -231,10 +231,10 @@ impl<'txin> Stack<'txin> {
     /// booleans
     pub(super) fn evaluate_after(
         &mut self,
-        n: &LockTime,
-        lock_time: LockTime,
+        n: &absolute::LockTime,
+        lock_time: absolute::LockTime,
     ) -> Option<Result<SatisfiedConstraint, Error>> {
-        use LockTime::*;
+        use absolute::LockTime::*;
 
         let is_satisfied = match (*n, lock_time) {
             (Blocks(n), Blocks(lock_time)) => n <= lock_time,
diff --git a/src/miniscript/astelem.rs b/src/miniscript/astelem.rs
@@ -23,7 +23,7 @@ use core::fmt;
 use core::str::FromStr;
 
 use bitcoin::blockdata::{opcodes, script};
-use bitcoin::{LockTime, Sequence};
+use bitcoin::{absolute, Sequence};
 use sync::Arc;
 
 use crate::miniscript::context::SigType;
@@ -460,7 +460,7 @@ impl_from_tree!(
             }
             ("pk_h", 1) => expression::terminal(&top.args[0], |x| Pk::from_str(x).map(Terminal::PkH)),
             ("after", 1) => expression::terminal(&top.args[0], |x| {
-                expression::parse_num(x).map(|x| Terminal::After(LockTime::from_consensus(x).into()))
+                expression::parse_num(x).map(|x| Terminal::After(absolute::LockTime::from_consensus(x).into()))
             }),
             ("older", 1) => expression::terminal(&top.args[0], |x| {
                 expression::parse_num(x).map(|x| Terminal::Older(Sequence::from_consensus(x)))
diff --git a/src/miniscript/decode.rs b/src/miniscript/decode.rs
@@ -26,7 +26,7 @@ use bitcoin::blockdata::constants::MAX_BLOCK_WEIGHT;
 use bitcoin::hashes::{hash160, ripemd160, sha256, Hash};
 use sync::Arc;
 
-use crate::bitcoin::{LockTime, PackedLockTime, Sequence};
+use crate::bitcoin::{absolute, Sequence};
 use crate::miniscript::lex::{Token as Tk, TokenIter};
 use crate::miniscript::limits::MAX_PUBKEYS_PER_MULTISIG;
 use crate::miniscript::types::extra_props::ExtData;
@@ -140,7 +140,7 @@ pub enum Terminal<Pk: MiniscriptKey, Ctx: ScriptContext> {
     RawPkH(Pk::RawPkHash),
     // timelocks
     /// `n CHECKLOCKTIMEVERIFY`
-    After(PackedLockTime),
+    After(absolute::PackedLockTime),
     /// `n CHECKSEQUENCEVERIFY`
     Older(Sequence),
     // hashlocks
@@ -395,7 +395,7 @@ pub fn parse<Ctx: ScriptContext>(
                     Tk::CheckSequenceVerify, Tk::Num(n)
                         => term.reduce0(Terminal::Older(Sequence::from_consensus(n)))?,
                     Tk::CheckLockTimeVerify, Tk::Num(n)
-                        => term.reduce0(Terminal::After(LockTime::from_consensus(n).into()))?,
+                        => term.reduce0(Terminal::After(absolute::LockTime::from_consensus(n).into()))?,
                     // hashlocks
                     Tk::Equal => match_token!(
                         tokens,
diff --git a/src/miniscript/satisfy.rs b/src/miniscript/satisfy.rs
@@ -22,7 +22,7 @@ use core::{cmp, i64, mem};
 
 use bitcoin::secp256k1::XOnlyPublicKey;
 use bitcoin::util::taproot::{ControlBlock, LeafVersion, TapLeafHash};
-use bitcoin::{LockTime, Sequence};
+use bitcoin::{absolute, relative, Sequence};
 use sync::Arc;
 
 use crate::prelude::*;
@@ -106,12 +106,12 @@ pub trait Satisfier<Pk: MiniscriptKey + ToPublicKey> {
     }
 
     /// Assert whether an relative locktime is satisfied
-    fn check_older(&self, _: Sequence) -> bool {
+    fn check_older(&self, _: relative::LockTime) -> bool {
         false
     }
 
     /// Assert whether a absolute locktime is satisfied
-    fn check_after(&self, _: LockTime) -> bool {
+    fn check_after(&self, _: absolute::LockTime) -> bool {
         false
     }
 }
@@ -120,32 +120,18 @@ pub trait Satisfier<Pk: MiniscriptKey + ToPublicKey> {
 impl<Pk: MiniscriptKey + ToPublicKey> Satisfier<Pk> for () {}
 
 impl<Pk: MiniscriptKey + ToPublicKey> Satisfier<Pk> for Sequence {
-    fn check_older(&self, n: Sequence) -> bool {
+    fn check_older(&self, n: relative::LockTime) -> bool {
         if !self.is_relative_lock_time() {
             return false;
         }
 
-        // We need a relative lock time type in rust-bitcoin to clean this up.
-
-        /* If nSequence encodes a relative lock-time, this mask is
-         * applied to extract that lock-time from the sequence field. */
-        const SEQUENCE_LOCKTIME_MASK: u32 = 0x0000ffff;
-        const SEQUENCE_LOCKTIME_TYPE_FLAG: u32 = 0x00400000;
-
-        let mask = SEQUENCE_LOCKTIME_MASK | SEQUENCE_LOCKTIME_TYPE_FLAG;
-        let masked_n = n.to_consensus_u32() & mask;
-        let masked_seq = self.to_consensus_u32() & mask;
-        if masked_n < SEQUENCE_LOCKTIME_TYPE_FLAG && masked_seq >= SEQUENCE_LOCKTIME_TYPE_FLAG {
-            false
-        } else {
-            masked_n <= masked_seq
-        }
+        n.is_satisfied_by(*self)
     }
 }
 
-impl<Pk: MiniscriptKey + ToPublicKey> Satisfier<Pk> for LockTime {
-    fn check_after(&self, n: LockTime) -> bool {
-        use LockTime::*;
+impl<Pk: MiniscriptKey + ToPublicKey> Satisfier<Pk> for absolute::LockTime {
+    fn check_after(&self, n: absolute::LockTime) -> bool {
+        use absolute::LockTime::*;
 
         match (n, *self) {
             (Blocks(n), Blocks(lock_time)) => n <= lock_time,
@@ -265,11 +251,11 @@ impl<'a, Pk: MiniscriptKey + ToPublicKey, S: Satisfier<Pk>> Satisfier<Pk> for &'
         (**self).lookup_hash160(h)
     }
 
-    fn check_older(&self, t: Sequence) -> bool {
+    fn check_older(&self, t: relative::LockTime) -> bool {
         (**self).check_older(t)
     }
 
-    fn check_after(&self, n: LockTime) -> bool {
+    fn check_after(&self, n: absolute::LockTime) -> bool {
         (**self).check_after(n)
     }
 }
@@ -327,11 +313,11 @@ impl<'a, Pk: MiniscriptKey + ToPublicKey, S: Satisfier<Pk>> Satisfier<Pk> for &'
         (**self).lookup_hash160(h)
     }
 
-    fn check_older(&self, t: Sequence) -> bool {
+    fn check_older(&self, t: relative::LockTime) -> bool {
         (**self).check_older(t)
     }
 
-    fn check_after(&self, n: LockTime) -> bool {
+    fn check_after(&self, n: absolute::LockTime) -> bool {
         (**self).check_after(n)
     }
 }
@@ -465,7 +451,7 @@ macro_rules! impl_tuple_satisfier {
                 None
             }
 
-            fn check_older(&self, n: Sequence) -> bool {
+            fn check_older(&self, n: relative::LockTime) -> bool {
                 let &($(ref $ty,)*) = self;
                 $(
                     if $ty.check_older(n) {
@@ -475,7 +461,7 @@ macro_rules! impl_tuple_satisfier {
                 false
             }
 
-            fn check_after(&self, n: LockTime) -> bool {
+            fn check_after(&self, n: absolute::LockTime) -> bool {
                 let &($(ref $ty,)*) = self;
                 $(
                     if $ty.check_after(n) {
@@ -953,21 +939,31 @@ impl Satisfaction {
                 },
                 has_sig: false,
             },
-            Terminal::Older(t) => Satisfaction {
-                stack: if stfr.check_older(t) {
-                    Witness::empty()
-                } else if root_has_sig {
-                    // If the root terminal has signature, the
-                    // signature covers the nLockTime and nSequence
-                    // values. The sender of the transaction should
-                    // take care that it signs the value such that the
-                    // timelock is not met
-                    Witness::Impossible
-                } else {
-                    Witness::Unavailable
-                },
+            Terminal::Older(t) => {
+                let older = {
+                    match t.to_relative_lock_time() {
+                        // https://github.com/bitcoin/bips/blob/master/bip-0112.mediawiki
+                        // Disable flag set => return true.
+                        None => true,
+                        Some(lock) => stfr.check_older(lock),
+                    }
+                };
+                Satisfaction {
+                    stack: if older {
+                        Witness::empty()
+                    } else if root_has_sig {
+                        // If the root terminal has signature, the
+                        // signature covers the nLockTime and nSequence
+                        // values. The sender of the transaction should
+                        // take care that it signs the value such that the
+                        // timelock is not met
+                        Witness::Impossible
+                    } else {
+                        Witness::Unavailable
+                    },
 
-                has_sig: false,
+                    has_sig: false,
+                }
             },
             Terminal::Ripemd160(ref h) => Satisfaction {
                 stack: Witness::ripemd160_preimage(stfr, h),
diff --git a/src/miniscript/types/extra_props.rs b/src/miniscript/types/extra_props.rs
@@ -4,7 +4,7 @@
 use core::cmp;
 use core::iter::once;
 
-use bitcoin::{LockTime, PackedLockTime, Sequence};
+use bitcoin::{absolute, Sequence};
 
 use super::{Error, ErrorKind, Property, ScriptContext};
 use crate::miniscript::context::SigType;
@@ -337,7 +337,7 @@ impl Property for ExtData {
         unreachable!()
     }
 
-    fn from_after(t: LockTime) -> Self {
+    fn from_after(t: absolute::LockTime) -> Self {
         ExtData {
             pk_cost: script_num_size(t.to_consensus_u32() as usize) + 1,
             has_free_verify: false,
@@ -933,7 +933,7 @@ impl Property for ExtData {
                 // Note that for CLTV this is a limitation not of Bitcoin but Miniscript. The
                 // number on the stack would be a 5 bytes signed integer but Miniscript's B type
                 // only consumes 4 bytes from the stack.
-                if t == PackedLockTime::ZERO {
+                if t == absolute::PackedLockTime::ZERO {
                     return Err(Error {
                         fragment: fragment.clone(),
                         error: ErrorKind::InvalidTime,
diff --git a/src/miniscript/types/mod.rs b/src/miniscript/types/mod.rs
@@ -24,7 +24,7 @@ use core::fmt;
 #[cfg(feature = "std")]
 use std::error;
 
-use bitcoin::{LockTime, PackedLockTime, Sequence};
+use bitcoin::{absolute, Sequence};
 
 pub use self::correctness::{Base, Correctness, Input};
 pub use self::extra_props::ExtData;
@@ -304,7 +304,7 @@ pub trait Property: Sized {
 
     /// Type property of an absolute timelock. Default implementation simply
     /// passes through to `from_time`
-    fn from_after(t: LockTime) -> Self {
+    fn from_after(t: absolute::LockTime) -> Self {
         Self::from_time(t.to_consensus_u32())
     }
 
@@ -438,7 +438,7 @@ pub trait Property: Sized {
                 // Note that for CLTV this is a limitation not of Bitcoin but Miniscript. The
                 // number on the stack would be a 5 bytes signed integer but Miniscript's B type
                 // only consumes 4 bytes from the stack.
-                if t == PackedLockTime::ZERO {
+                if t == absolute::PackedLockTime::ZERO {
                     return Err(Error {
                         fragment: fragment.clone(),
                         error: ErrorKind::InvalidTime,
@@ -625,7 +625,7 @@ impl Property for Type {
         }
     }
 
-    fn from_after(t: LockTime) -> Self {
+    fn from_after(t: absolute::LockTime) -> Self {
         Type {
             corr: Property::from_after(t),
             mall: Property::from_after(t),
@@ -821,7 +821,7 @@ impl Property for Type {
                 // Note that for CLTV this is a limitation not of Bitcoin but Miniscript. The
                 // number on the stack would be a 5 bytes signed integer but Miniscript's B type
                 // only consumes 4 bytes from the stack.
-                if t == PackedLockTime::ZERO {
+                if t == absolute::PackedLockTime::ZERO {
                     return Err(Error {
                         fragment: fragment.clone(),
                         error: ErrorKind::InvalidTime,
diff --git a/src/policy/concrete.rs b/src/policy/concrete.rs
diff --git a/src/policy/semantic.rs b/src/policy/semantic.rs
diff --git a/src/psbt/mod.rs b/src/psbt/mod.rs
