WIP: Use new LockTime type

Author: tcharding

Cargo.toml

@@ -17,18 +17,19 @@ no-std = ["hashbrown", "bitcoin/no-std"]
 compiler = []
 trace = []
 unstable = []
-use-serde = ["serde", "bitcoin/use-serde"]
+use-serde = ["serde", "bitcoin/serde"]
 rand = ["bitcoin/rand"]
 
 [dependencies]
-bitcoin = { version = "0.28.1", default-features = false }
+bitcoin = { path = "../rust-bitcoin", default-features = false }
 serde = { version = "1.0", optional = true }
 hashbrown = { version = "0.11", optional = true }
 
 [dev-dependencies]
-bitcoind = {version = "0.26.1", features=["22_0"]}
+bitcoind = { path = "../../RCasatta/bitcoind", features=["22_0"] }
 actual-rand = { package = "rand", version = "0.8.4"}
 secp256k1 = {version = "0.22.1", features = ["rand-std"]}
+bitcoin = { path = "../rust-bitcoin", features = ["rand"] }
 
 [[example]]
 name = "htlc"

examples/sign_multisig.rs

@@ -17,8 +17,8 @@
 use std::collections::HashMap;
 use std::str::FromStr;
 
+use bitcoin::{secp256k1, LockTime};
 use bitcoin::blockdata::witness::Witness;
-use bitcoin::secp256k1;
 
 fn main() {
     let mut tx = spending_transaction();
@@ -91,7 +91,7 @@ fn main() {
 fn spending_transaction() -> bitcoin::Transaction {
     bitcoin::Transaction {
         version: 2,
-        lock_time: 0,
+        lock_time: LockTime::ZERO,
         input: vec![bitcoin::TxIn {
             previous_output: Default::default(),
             script_sig: bitcoin::Script::new(),

examples/verify_tx.rs

@@ -16,6 +16,7 @@
 
 use std::str::FromStr;
 
+use bitcoin::LockTime;
 use bitcoin::consensus::Decodable;
 use bitcoin::secp256k1::{self, Secp256k1};
 use bitcoin::util::sighash;
@@ -34,7 +35,7 @@ fn main() {
         &tx.input[0].script_sig,
         &tx.input[0].witness,
         0,
-        0,
+        LockTime::ZERO,
     )
     .unwrap();
 

src/descriptor/bare.rs

@@ -36,7 +36,7 @@ use crate::{
 
 /// Create a Bare Descriptor. That is descriptor that is
 /// not wrapped in sh or wsh. This covers the Pk descriptor
-#[derive(Clone, Ord, PartialOrd, Eq, PartialEq, Hash)]
+#[derive(Clone, PartialOrd, Eq, PartialEq, Hash)]
 pub struct Bare<Pk: MiniscriptKey> {
     /// underlying miniscript
     ms: Miniscript<Pk, BareCtx>,

src/descriptor/mod.rs

@@ -71,7 +71,7 @@ pub use self::key::{
 pub type KeyMap = HashMap<DescriptorPublicKey, DescriptorSecretKey>;
 
 /// Script descriptor
-#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[derive(Clone, PartialEq, Eq, PartialOrd, Hash)]
 pub enum Descriptor<Pk: MiniscriptKey> {
     /// A raw scriptpubkey (including pay-to-pubkey) under Legacy context
     Bare(Bare<Pk>),

src/descriptor/segwitv0.rs

@@ -32,7 +32,7 @@ use crate::{
     Translator,
 };
 /// A Segwitv0 wsh descriptor
-#[derive(Clone, Ord, PartialOrd, Eq, PartialEq, Hash)]
+#[derive(Clone, PartialOrd, Eq, PartialEq, Hash)]
 pub struct Wsh<Pk: MiniscriptKey> {
     /// underlying miniscript
     inner: WshInner<Pk>,
@@ -176,7 +176,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Wsh<Pk> {
 }
 
 /// Wsh Inner
-#[derive(Clone, Ord, PartialOrd, Eq, PartialEq, Hash)]
+#[derive(Clone, PartialOrd, Eq, PartialEq, Hash)]
 pub enum WshInner<Pk: MiniscriptKey> {
     /// Sorted Multi
     SortedMulti(SortedMultiVec<Pk, Segwitv0>),

src/descriptor/sh.rs

@@ -36,14 +36,14 @@ use crate::{
 };
 
 /// A Legacy p2sh Descriptor
-#[derive(Clone, Ord, PartialOrd, Eq, PartialEq, Hash)]
+#[derive(Clone, PartialOrd, Eq, PartialEq, Hash)]
 pub struct Sh<Pk: MiniscriptKey> {
     /// underlying miniscript
     inner: ShInner<Pk>,
 }
 
 /// Sh Inner
-#[derive(Clone, Ord, PartialOrd, Eq, PartialEq, Hash)]
+#[derive(Clone, PartialOrd, Eq, PartialEq, Hash)]
 pub enum ShInner<Pk: MiniscriptKey> {
     /// Nested Wsh
     Wsh(Wsh<Pk>),

src/descriptor/tr.rs

@@ -25,7 +25,7 @@ use crate::{
 /// A Taproot Tree representation.
 // Hidden leaves are not yet supported in descriptor spec. Conceptually, it should
 // be simple to integrate those here, but it is best to wait on core for the exact syntax.
-#[derive(Clone, Ord, PartialOrd, Eq, PartialEq, Hash)]
+#[derive(Clone, PartialOrd, Eq, PartialEq, Hash)]
 pub enum TapTree<Pk: MiniscriptKey> {
     /// A taproot tree structure
     Tree(Arc<TapTree<Pk>>, Arc<TapTree<Pk>>),
@@ -89,16 +89,6 @@ impl<Pk: MiniscriptKey> PartialOrd for Tr<Pk> {
     }
 }
 
-impl<Pk: MiniscriptKey> Ord for Tr<Pk> {
-    fn cmp(&self, other: &Self) -> cmp::Ordering {
-        match self.internal_key.cmp(&other.internal_key) {
-            cmp::Ordering::Equal => {}
-            ord => return ord,
-        }
-        self.tree.cmp(&other.tree)
-    }
-}
-
 impl<Pk: MiniscriptKey> hash::Hash for Tr<Pk> {
     fn hash<H: hash::Hasher>(&self, state: &mut H) {
         self.internal_key.hash(state);
@@ -257,6 +247,7 @@ impl<Pk: MiniscriptKey> Tr<Pk> {
                     TaprootBuilderError::EmptyTree => {
                         unreachable!("Taptree is a well formed tree with atleast 1 element")
                     }
+                    _ => unreachable!("non_exhaustive catchall")
                 },
             }
         };

src/interpreter/error.rs

@@ -29,6 +29,8 @@ use crate::prelude::*;
 pub enum Error {
     /// Could not satisfy, absolute locktime not met
     AbsoluteLocktimeNotMet(u32),
+    /// Could not satisfy, lock time values are different units
+    AbsoluteLocktimeComparisonInvalid(u32, u32),
     /// 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
@@ -187,6 +189,7 @@ impl fmt::Display for Error {
             Error::VerifyFailed => {
                 f.write_str("Expected Satisfied Boolean at stack top for VERIFY")
             }
+            _ => f.write_str("Unknown error, non_exhaustive catch all"),
         }
     }
 }
@@ -234,6 +237,7 @@ impl error::Error for Error {
             Secp(e) => Some(e),
             SchnorrSig(e) => Some(e),
             SighashError(e) => Some(e),
+            _ => None // non_exhaustive catch all.
         }
     }
 }

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, TxOut};
+use bitcoin::{self, secp256k1, TxOut, LockTime};
 
 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: u32,
-    lock_time: u32,
+    lock_time: LockTime,
 }
 
 // A type representing functions for checking signatures that accept both
@@ -173,8 +173,8 @@ impl<'txin> Interpreter<'txin> {
         spk: &bitcoin::Script,
         script_sig: &'txin bitcoin::Script,
         witness: &'txin Witness,
-        age: u32,       // CSV, relative lock time.
-        lock_time: u32, // CLTV, absolute lock time.
+        age: u32,               // CSV, relative lock time.
+        lock_time: 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
-        time: u32,
+        n: LockTime,
     },
 }
 
@@ -532,7 +532,7 @@ pub struct Iter<'intp, 'txin: 'intp> {
     state: Vec<NodeEvaluationState<'intp>>,
     stack: Stack<'txin>,
     age: u32,
-    lock_time: u32,
+    lock_time: LockTime,
     has_errored: bool,
 }
 
@@ -1145,7 +1145,7 @@ mod tests {
                     n_satisfied: 0,
                 }],
                 age: 1002,
-                lock_time: 1002,
+                lock_time: LockTime::from_consensus(1002),
                 has_errored: false,
             }
         }
@@ -1208,7 +1208,7 @@ mod tests {
         let after_satisfied: Result<Vec<SatisfiedConstraint>, Error> = constraints.collect();
         assert_eq!(
             after_satisfied.unwrap(),
-            vec![SatisfiedConstraint::AbsoluteTimelock { time: 1000 }]
+            vec![SatisfiedConstraint::AbsoluteTimelock { n: LockTime::from_consensus(1000) }]
         );
 
         //Check Older

src/interpreter/stack.rs

@@ -17,6 +17,7 @@
 use bitcoin;
 use bitcoin::blockdata::{opcodes, script};
 use bitcoin::hashes::{hash160, ripemd160, sha256, Hash};
+use bitcoin::LockTime;
 
 use super::error::PkEvalErrInner;
 use super::{
@@ -230,14 +231,22 @@ impl<'txin> Stack<'txin> {
     /// booleans
     pub(super) fn evaluate_after(
         &mut self,
-        n: &u32,
-        lock_time: u32,
+        n: &LockTime,
+        lock_time: LockTime,
     ) -> Option<Result<SatisfiedConstraint, Error>> {
-        if lock_time >= *n {
+        use LockTime::*;
+
+        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()))),
+        };
+
+        if is_satisfied {
             self.push(Element::Satisfied);
-            Some(Ok(SatisfiedConstraint::AbsoluteTimelock { time: *n }))
+            Some(Ok(SatisfiedConstraint::AbsoluteTimelock { n: *n }))
         } else {
-            Some(Err(Error::AbsoluteLocktimeNotMet(*n)))
+            Some(Err(Error::AbsoluteLocktimeNotMet(n.to_consensus_u32())))
         }
     }
 

src/lib.rs

@@ -116,7 +116,6 @@ pub mod interpreter;
 pub mod miniscript;
 pub mod policy;
 pub mod psbt;
-pub mod timelock;
 
 #[cfg(test)]
 mod test_utils;

src/miniscript/astelem.rs

@@ -22,6 +22,7 @@
 use core::fmt;
 use core::str::FromStr;
 
+use bitcoin::LockTime;
 use bitcoin::blockdata::{opcodes, script};
 use sync::Arc;
 
@@ -459,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(Terminal::After)
+                expression::parse_num(x).map(|x| Terminal::After(LockTime::from_consensus(x)))
             }),
             ("older", 1) => expression::terminal(&top.args[0], |x| {
                 expression::parse_num(x).map(Terminal::Older)
@@ -620,7 +621,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Terminal<Pk, Ctx> {
                 .push_slice(&Pk::hash_to_hash160(hash)[..])
                 .push_opcode(opcodes::all::OP_EQUALVERIFY),
             Terminal::After(t) => builder
-                .push_int(t as i64)
+                .push_int(t.to_consensus_u32().into())
                 .push_opcode(opcodes::all::OP_CLTV),
             Terminal::Older(t) => builder.push_int(t as i64).push_opcode(opcodes::all::OP_CSV),
             Terminal::Sha256(ref h) => builder
@@ -755,7 +756,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Terminal<Pk, Ctx> {
         match *self {
             Terminal::PkK(ref pk) => Ctx::pk_len(pk),
             Terminal::PkH(..) | Terminal::RawPkH(..) => 24,
-            Terminal::After(n) => script_num_size(n as usize) + 1,
+            Terminal::After(n) => script_num_size(n.to_consensus_u32() as usize) + 1,
             Terminal::Older(n) => script_num_size(n as usize) + 1,
             Terminal::Sha256(..) => 33 + 6,
             Terminal::Hash256(..) => 33 + 6,

src/miniscript/decode.rs

@@ -28,6 +28,7 @@ use sync::Arc;
 
 use crate::miniscript::lex::{Token as Tk, TokenIter};
 use crate::miniscript::limits::MAX_PUBKEYS_PER_MULTISIG;
+use crate::bitcoin::LockTime;
 use crate::miniscript::types::extra_props::ExtData;
 use crate::miniscript::types::{Property, Type};
 use crate::miniscript::ScriptContext;
@@ -124,7 +125,7 @@ enum NonTerm {
 }
 /// All AST elements
 #[allow(broken_intra_doc_links)]
-#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[derive(Clone, PartialEq, Eq, PartialOrd, Hash)]
 pub enum Terminal<Pk: MiniscriptKey, Ctx: ScriptContext> {
     /// `1`
     True,
@@ -139,7 +140,7 @@ pub enum Terminal<Pk: MiniscriptKey, Ctx: ScriptContext> {
     RawPkH(Pk::RawPkHash),
     // timelocks
     /// `n CHECKLOCKTIMEVERIFY`
-    After(u32),
+    After(LockTime),
     /// `n CHECKSEQUENCEVERIFY`
     Older(u32),
     // hashlocks
@@ -394,7 +395,7 @@ pub fn parse<Ctx: ScriptContext>(
                     Tk::CheckSequenceVerify, Tk::Num(n)
                         => term.reduce0(Terminal::Older(n))?,
                     Tk::CheckLockTimeVerify, Tk::Num(n)
-                        => term.reduce0(Terminal::After(n))?,
+                        => term.reduce0(Terminal::After(LockTime::from_consensus(n)))?,
                     // hashlocks
                     Tk::Equal => match_token!(
                         tokens,

src/miniscript/mod.rs

@@ -76,16 +76,7 @@ pub struct Miniscript<Pk: MiniscriptKey, Ctx: ScriptContext> {
 /// by the ast.
 impl<Pk: MiniscriptKey, Ctx: ScriptContext> PartialOrd for Miniscript<Pk, Ctx> {
     fn partial_cmp(&self, other: &Miniscript<Pk, Ctx>) -> Option<cmp::Ordering> {
-        Some(self.node.cmp(&other.node))
-    }
-}
-
-/// `Ord` of `Miniscript` must depend only on node and not the type information.
-/// The type information and extra_properties can be deterministically determined
-/// by the ast.
-impl<Pk: MiniscriptKey, Ctx: ScriptContext> Ord for Miniscript<Pk, Ctx> {
-    fn cmp(&self, other: &Miniscript<Pk, Ctx>) -> cmp::Ordering {
-        self.node.cmp(&other.node)
+        self.node.partial_cmp(&other.node)
     }
 }
 
@@ -980,7 +971,7 @@ mod tests {
         ));
         assert_eq!(
             ms.unwrap_err().to_string(),
-            "unexpected «Key hex decoding error: bad hex string length 64 (expected 66)»"
+            "unexpected «key hex decoding error»"
         );
         Tapscript::from_str_insane(&format!(
             "pk(2788ee41e76f4f3af603da5bc8fa22997bc0344bb0f95666ba6aaff0242baa99)"