From 8e58db9ac90535c3636ac7665f5769a0111b344d Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 17 Jun 2022 15:18:42 +1000 Subject: [PATCH] WIP: Use new LockTime type --- Cargo.toml | 7 +-- examples/sign_multisig.rs | 3 +- examples/verify_tx.rs | 3 +- src/descriptor/bare.rs | 2 +- src/descriptor/mod.rs | 2 +- src/descriptor/segwitv0.rs | 4 +- src/descriptor/sh.rs | 4 +- src/descriptor/tr.rs | 13 +---- src/interpreter/error.rs | 4 ++ src/interpreter/mod.rs | 16 +++--- src/interpreter/stack.rs | 19 +++++-- src/lib.rs | 1 - src/miniscript/astelem.rs | 7 +-- src/miniscript/decode.rs | 7 +-- src/miniscript/mod.rs | 13 +---- src/miniscript/satisfy.rs | 31 +++-------- src/miniscript/types/extra_props.rs | 14 ++--- src/miniscript/types/mod.rs | 14 ++--- src/policy/compiler.rs | 2 +- src/policy/concrete.rs | 35 ++++++++++--- src/policy/semantic.rs | 80 +++++++++++++++++------------ src/psbt/mod.rs | 30 ++++++----- src/timelock.rs | 21 -------- tests/test_cpp.rs | 10 ++-- tests/test_desc.rs | 8 +-- 25 files changed, 179 insertions(+), 171 deletions(-) delete mode 100644 src/timelock.rs diff --git a/Cargo.toml b/Cargo.toml index b3340376b..bab3e716d 100644 --- a/Cargo.toml +++ b/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" diff --git a/examples/sign_multisig.rs b/examples/sign_multisig.rs index 5c58bf5d2..a9b762904 100644 --- a/examples/sign_multisig.rs +++ b/examples/sign_multisig.rs @@ -18,6 +18,7 @@ use std::collections::HashMap; use std::str::FromStr; use bitcoin::blockdata::witness::Witness; +use bitcoin::blockdata::locktime; use bitcoin::secp256k1; fn main() { @@ -91,7 +92,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(), diff --git a/examples/verify_tx.rs b/examples/verify_tx.rs index dd8cc75ff..956d65a0a 100644 --- a/examples/verify_tx.rs +++ b/examples/verify_tx.rs @@ -16,6 +16,7 @@ use std::str::FromStr; +use bitcoin::blockdata::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(); diff --git a/src/descriptor/bare.rs b/src/descriptor/bare.rs index 9d5dbcc85..41f1bc718 100644 --- a/src/descriptor/bare.rs +++ b/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 { /// underlying miniscript ms: Miniscript, diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs index 2b02dc165..86eb76032 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -71,7 +71,7 @@ pub use self::key::{ pub type KeyMap = HashMap; /// Script descriptor -#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +#[derive(Clone, PartialEq, Eq, PartialOrd, Hash)] pub enum Descriptor { /// A raw scriptpubkey (including pay-to-pubkey) under Legacy context Bare(Bare), diff --git a/src/descriptor/segwitv0.rs b/src/descriptor/segwitv0.rs index 9f306ccbc..a3290d787 100644 --- a/src/descriptor/segwitv0.rs +++ b/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 { /// underlying miniscript inner: WshInner, @@ -176,7 +176,7 @@ impl Wsh { } /// Wsh Inner -#[derive(Clone, Ord, PartialOrd, Eq, PartialEq, Hash)] +#[derive(Clone, PartialOrd, Eq, PartialEq, Hash)] pub enum WshInner { /// Sorted Multi SortedMulti(SortedMultiVec), diff --git a/src/descriptor/sh.rs b/src/descriptor/sh.rs index 3b387fc85..d14b5e963 100644 --- a/src/descriptor/sh.rs +++ b/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 { /// underlying miniscript inner: ShInner, } /// Sh Inner -#[derive(Clone, Ord, PartialOrd, Eq, PartialEq, Hash)] +#[derive(Clone, PartialOrd, Eq, PartialEq, Hash)] pub enum ShInner { /// Nested Wsh Wsh(Wsh), diff --git a/src/descriptor/tr.rs b/src/descriptor/tr.rs index 7280893f7..cede84dd7 100644 --- a/src/descriptor/tr.rs +++ b/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 { /// A taproot tree structure Tree(Arc>, Arc>), @@ -89,16 +89,6 @@ impl PartialOrd for Tr { } } -impl Ord for Tr { - 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 hash::Hash for Tr { fn hash(&self, state: &mut H) { self.internal_key.hash(state); @@ -257,6 +247,7 @@ impl Tr { TaprootBuilderError::EmptyTree => { unreachable!("Taptree is a well formed tree with atleast 1 element") } + _ => unreachable!("non_exhaustive catchall") }, } }; diff --git a/src/interpreter/error.rs b/src/interpreter/error.rs index 65fe2b848..f795acbfe 100644 --- a/src/interpreter/error.rs +++ b/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. } } } diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index 2c2c4b917..2a47823b3 100644 --- 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, 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, 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 { 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>, 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, Error> = constraints.collect(); assert_eq!( after_satisfied.unwrap(), - vec![SatisfiedConstraint::AbsoluteTimelock { time: 1000 }] + vec![SatisfiedConstraint::AbsoluteTimelock { n: LockTime::from_consensus(1000) }] ); //Check Older diff --git a/src/interpreter/stack.rs b/src/interpreter/stack.rs index 6591a6afd..a74d594ae 100644 --- a/src/interpreter/stack.rs +++ b/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> { - 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()))) } } diff --git a/src/lib.rs b/src/lib.rs index dd0888624..2021e71a6 100644 --- a/src/lib.rs +++ b/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; diff --git a/src/miniscript/astelem.rs b/src/miniscript/astelem.rs index b21756430..802ff2216 100644 --- a/src/miniscript/astelem.rs +++ b/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 Terminal { .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() as i64) .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 Terminal { 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, diff --git a/src/miniscript/decode.rs b/src/miniscript/decode.rs index d7f72c1d8..b304ba518 100644 --- a/src/miniscript/decode.rs +++ b/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 { /// `1` True, @@ -139,7 +140,7 @@ pub enum Terminal { RawPkH(Pk::RawPkHash), // timelocks /// `n CHECKLOCKTIMEVERIFY` - After(u32), + After(LockTime), /// `n CHECKSEQUENCEVERIFY` Older(u32), // hashlocks @@ -394,7 +395,7 @@ pub fn parse( 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, diff --git a/src/miniscript/mod.rs b/src/miniscript/mod.rs index 9d50d63d1..b1287ac7d 100644 --- a/src/miniscript/mod.rs +++ b/src/miniscript/mod.rs @@ -76,16 +76,7 @@ pub struct Miniscript { /// by the ast. impl PartialOrd for Miniscript { fn partial_cmp(&self, other: &Miniscript) -> Option { - 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 Ord for Miniscript { - fn cmp(&self, other: &Miniscript) -> 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)" diff --git a/src/miniscript/satisfy.rs b/src/miniscript/satisfy.rs index 37723117c..c1e8d8895 100644 --- a/src/miniscript/satisfy.rs +++ b/src/miniscript/satisfy.rs @@ -20,13 +20,13 @@ use core::{cmp, i64, mem}; -use bitcoin; +use bitcoin::LockTime; use bitcoin::secp256k1::XOnlyPublicKey; use bitcoin::util::taproot::{ControlBlock, LeafVersion, TapLeafHash}; use sync::Arc; use crate::miniscript::limits::{ - LOCKTIME_THRESHOLD, SEQUENCE_LOCKTIME_DISABLE_FLAG, SEQUENCE_LOCKTIME_TYPE_FLAG, + SEQUENCE_LOCKTIME_DISABLE_FLAG, SEQUENCE_LOCKTIME_TYPE_FLAG, }; use crate::prelude::*; use crate::util::witness_size; @@ -114,7 +114,7 @@ pub trait Satisfier { } /// Assert whether a absolute locktime is satisfied - fn check_after(&self, _: u32) -> bool { + fn check_after(&self, _: LockTime) -> bool { false } } @@ -147,21 +147,6 @@ impl Satisfier for Older { } } -/// Newtype around `u32` which implements `Satisfier` using `n` as an -/// absolute locktime -pub struct After(pub u32); - -impl Satisfier for After { - fn check_after(&self, n: u32) -> bool { - // if n > self.0; we will be returning false anyways - if n < LOCKTIME_THRESHOLD && self.0 >= LOCKTIME_THRESHOLD { - false - } else { - n <= self.0 - } - } -} - impl Satisfier for HashMap { fn lookup_ecdsa_sig(&self, key: &Pk) -> Option { self.get(key).copied() @@ -277,8 +262,8 @@ impl<'a, Pk: MiniscriptKey + ToPublicKey, S: Satisfier> Satisfier for &' (**self).check_older(t) } - fn check_after(&self, t: u32) -> bool { - (**self).check_after(t) + fn check_after(&self, n: LockTime) -> bool { + (**self).check_after(n) } } @@ -339,8 +324,8 @@ impl<'a, Pk: MiniscriptKey + ToPublicKey, S: Satisfier> Satisfier for &' (**self).check_older(t) } - fn check_after(&self, t: u32) -> bool { - (**self).check_after(t) + fn check_after(&self, n: LockTime) -> bool { + (**self).check_after(n) } } @@ -483,7 +468,7 @@ macro_rules! impl_tuple_satisfier { false } - fn check_after(&self, n: u32) -> bool { + fn check_after(&self, n: LockTime) -> bool { let &($(ref $ty,)*) = self; $( if $ty.check_after(n) { diff --git a/src/miniscript/types/extra_props.rs b/src/miniscript/types/extra_props.rs index 0cd97f78b..b80683bd8 100644 --- a/src/miniscript/types/extra_props.rs +++ b/src/miniscript/types/extra_props.rs @@ -4,10 +4,12 @@ use core::cmp; use core::iter::once; +use bitcoin::{locktime, LockTime}; + use super::{Error, ErrorKind, Property, ScriptContext}; use crate::miniscript::context::SigType; use crate::miniscript::limits::{ - LOCKTIME_THRESHOLD, SEQUENCE_LOCKTIME_DISABLE_FLAG, SEQUENCE_LOCKTIME_TYPE_FLAG, + SEQUENCE_LOCKTIME_DISABLE_FLAG, SEQUENCE_LOCKTIME_TYPE_FLAG, }; use crate::prelude::*; use crate::{script_num_size, MiniscriptKey, Terminal}; @@ -338,9 +340,9 @@ impl Property for ExtData { unreachable!() } - fn from_after(t: u32) -> Self { + fn from_after(t: LockTime) -> Self { ExtData { - pk_cost: script_num_size(t as usize) + 1, + pk_cost: script_num_size(t.to_consensus_u32() as usize) + 1, has_free_verify: false, ops: OpLimits::new(1, Some(0), None), stack_elem_count_sat: Some(0), @@ -350,8 +352,8 @@ impl Property for ExtData { timelock_info: TimelockInfo { csv_with_height: false, csv_with_time: false, - cltv_with_height: t < LOCKTIME_THRESHOLD, - cltv_with_time: t >= LOCKTIME_THRESHOLD, + cltv_with_height: t.is_block_height(), + cltv_with_time: t.is_block_time(), contains_combination: false, }, exec_stack_elem_count_sat: Some(1), // @@ -934,7 +936,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 == 0 || (t & SEQUENCE_LOCKTIME_DISABLE_FLAG) != 0 { + if t == locktime::ZERO || (t.to_consensus_u32() & SEQUENCE_LOCKTIME_DISABLE_FLAG) != 0 { return Err(Error { fragment: fragment.clone(), error: ErrorKind::InvalidTime, diff --git a/src/miniscript/types/mod.rs b/src/miniscript/types/mod.rs index 161185c4f..a8e2140d8 100644 --- a/src/miniscript/types/mod.rs +++ b/src/miniscript/types/mod.rs @@ -24,6 +24,8 @@ use core::fmt; #[cfg(feature = "std")] use std::error; +use bitcoin::locktime::{self, LockTime}; + pub use self::correctness::{Base, Correctness, Input}; pub use self::extra_props::ExtData; pub use self::malleability::{Dissat, Malleability}; @@ -98,7 +100,7 @@ pub enum ErrorKind { } /// Error type for typechecking -#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)] +#[derive(Clone, PartialEq, Eq, PartialOrd, Hash, Debug)] pub struct Error { /// The fragment that failed typecheck pub fragment: Terminal, @@ -303,8 +305,8 @@ pub trait Property: Sized { /// Type property of an absolute timelock. Default implementation simply /// passes through to `from_time` - fn from_after(t: u32) -> Self { - Self::from_time(t) + fn from_after(t: LockTime) -> Self { + Self::from_time(t.to_consensus_u32()) } /// Type property of a relative timelock. Default implementation simply @@ -437,7 +439,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 == 0 || (t & SEQUENCE_LOCKTIME_DISABLE_FLAG) != 0 { + if t == locktime::ZERO || (t.to_consensus_u32() & SEQUENCE_LOCKTIME_DISABLE_FLAG) != 0 { return Err(Error { fragment: fragment.clone(), error: ErrorKind::InvalidTime, @@ -624,7 +626,7 @@ impl Property for Type { } } - fn from_after(t: u32) -> Self { + fn from_after(t: LockTime) -> Self { Type { corr: Property::from_after(t), mall: Property::from_after(t), @@ -820,7 +822,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 == 0 || (t & SEQUENCE_LOCKTIME_DISABLE_FLAG) != 0 { + if t == locktime::ZERO || (t.to_consensus_u32() & SEQUENCE_LOCKTIME_DISABLE_FLAG) != 0 { return Err(Error { fragment: fragment.clone(), error: ErrorKind::InvalidTime, diff --git a/src/policy/compiler.rs b/src/policy/compiler.rs index c5cb7cdea..0ae6c99df 100644 --- a/src/policy/compiler.rs +++ b/src/policy/compiler.rs @@ -1255,7 +1255,7 @@ mod tests { // artificially create a policy that is problematic and try to compile let pol: SPolicy = Concrete::And(vec![ Concrete::Key("A".to_string()), - Concrete::And(vec![Concrete::After(9), Concrete::After(1000_000_000)]), + Concrete::And(vec![Concrete::after(9), Concrete::after(1000_000_000)]), ]); assert!(pol.compile::().is_err()); diff --git a/src/policy/concrete.rs b/src/policy/concrete.rs index 6e28f3d11..97db70a35 100644 --- a/src/policy/concrete.rs +++ b/src/policy/concrete.rs @@ -19,6 +19,7 @@ use core::{fmt, str}; #[cfg(feature = "std")] use std::error; +use bitcoin::{locktime, LockTime}; #[cfg(feature = "compiler")] use { crate::descriptor::TapTree, @@ -35,7 +36,7 @@ use { use super::ENTAILMENT_MAX_TERMINALS; use crate::expression::{self, FromTree}; -use crate::miniscript::limits::{LOCKTIME_THRESHOLD, SEQUENCE_LOCKTIME_TYPE_FLAG}; +use crate::miniscript::limits::{SEQUENCE_LOCKTIME_TYPE_FLAG}; use crate::miniscript::types::extra_props::TimelockInfo; use crate::prelude::*; use crate::{errstr, Error, ForEachKey, MiniscriptKey, Translator}; @@ -43,7 +44,7 @@ use crate::{errstr, Error, ForEachKey, MiniscriptKey, Translator}; /// Concrete policy which corresponds directly to a Miniscript structure, /// and whose disjunctions are annotated with satisfaction probabilities /// to assist the compiler -#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +#[derive(Clone, PartialEq, Eq, PartialOrd, Hash)] pub enum Policy { /// Unsatisfiable Unsatisfiable, @@ -52,7 +53,7 @@ pub enum Policy { /// A public key which must sign to satisfy the descriptor Key(Pk), /// An absolute locktime restriction - After(u32), + After(LockTime), /// A relative locktime restriction Older(u32), /// A SHA256 whose preimage must be provided to satisfy the descriptor @@ -72,6 +73,17 @@ pub enum Policy { Threshold(usize, Vec>), } +impl Policy +where + Pk: MiniscriptKey +{ + /// Construct a `Policy::After` from `n`. Helper function equivalent to + /// `Policy::After(LockTime::from_consensus(n))`. + pub fn after(n: u32) -> Policy { + Policy::After(LockTime::from_consensus(n)) + } +} + /// Detailed Error type for Policies #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub enum PolicyError { @@ -548,8 +560,8 @@ impl Policy { Policy::After(t) => TimelockInfo { csv_with_height: false, csv_with_time: false, - cltv_with_height: t < LOCKTIME_THRESHOLD, - cltv_with_time: t >= LOCKTIME_THRESHOLD, + cltv_with_height: t.is_block_height(), + cltv_with_time: t.is_block_time(), contains_combination: false, }, Policy::Older(t) => TimelockInfo { @@ -614,7 +626,16 @@ impl Policy { Ok(()) } } - Policy::After(n) | Policy::Older(n) => { + Policy::After(n) => { + if n == locktime::ZERO { + Err(PolicyError::ZeroTime) + } else if n.to_consensus_u32() > 2u32.pow(31) { + Err(PolicyError::TimeTooFar) + } else { + Ok(()) + } + }, + Policy::Older(n) => { if n == 0 { Err(PolicyError::ZeroTime) } else if n > 2u32.pow(31) { @@ -824,7 +845,7 @@ impl_block_str!( } else if num == 0 { return Err(Error::PolicyError(PolicyError::ZeroTime)); } - Ok(Policy::After(num)) + Ok(Policy::After(LockTime::from_consensus(num))) } ("older", 1) => { let num = expression::terminal(&top.args[0], expression::parse_num)?; diff --git a/src/policy/semantic.rs b/src/policy/semantic.rs index 1bf430cb9..5cdc28a39 100644 --- a/src/policy/semantic.rs +++ b/src/policy/semantic.rs @@ -16,11 +16,14 @@ use core::str::FromStr; use core::{fmt, str}; +use core::cmp::Ordering; + +use bitcoin::LockTime; use super::concrete::PolicyError; use super::ENTAILMENT_MAX_TERMINALS; use crate::prelude::*; -use crate::{errstr, expression, timelock, Error, ForEachKey, MiniscriptKey, Translator}; +use crate::{errstr, expression, Error, ForEachKey, MiniscriptKey, Translator}; /// Abstract policy which corresponds to the semantics of a Miniscript /// and which allows complex forms of analysis, e.g. filtering and @@ -28,7 +31,7 @@ use crate::{errstr, expression, timelock, Error, ForEachKey, MiniscriptKey, Tran /// Semantic policies store only hashes of keys to ensure that objects /// representing the same policy are lifted to the same `Semantic`, /// regardless of their choice of `pk` or `pk_h` nodes. -#[derive(Clone, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Clone, PartialEq, PartialOrd, Eq)] pub enum Policy { /// Unsatisfiable Unsatisfiable, @@ -37,7 +40,7 @@ pub enum Policy { /// Signature and public key matching a given hash is required KeyHash(Pk::RawPkHash), /// An absolute locktime restriction - After(u32), + After(LockTime), /// A relative locktime restriction Older(u32), /// A SHA256 whose preimage must be provided to satisfy the descriptor @@ -52,6 +55,17 @@ pub enum Policy { Threshold(usize, Vec>), } +impl Policy +where + Pk: MiniscriptKey +{ + /// Construct a `Policy::After` from `n`. Helper function equivalent to + /// `Policy::After(LockTime::from_consensus(n))`. + pub fn after(n: u32) -> Policy { + Policy::After(LockTime::from_consensus(n)) + } +} + impl ForEachKey for Policy { fn for_each_key<'a, F: FnMut(&'a Pk) -> bool>(&'a self, mut pred: F) -> bool where @@ -345,7 +359,7 @@ impl_from_tree!( Pk::RawPkHash::from_str(pk).map(Policy::KeyHash) }), ("after", 1) => expression::terminal(&top.args[0], |x| { - expression::parse_num(x).map(Policy::After) + expression::parse_num(x).map(|x| Policy::After(LockTime::from_consensus(x))) }), ("older", 1) => expression::terminal(&top.args[0], |x| { expression::parse_num(x).map(Policy::Older) @@ -531,7 +545,7 @@ impl Policy { | Policy::Ripemd160(..) | Policy::Hash160(..) => vec![], Policy::Older(..) => vec![], - Policy::After(t) => vec![t], + Policy::After(t) => vec![t.to_consensus_u32()], Policy::Threshold(_, ref subs) => subs.iter().fold(vec![], |mut acc, x| { acc.extend(x.real_absolute_timelocks()); acc @@ -569,10 +583,10 @@ impl Policy { /// Filter a policy by eliminating absolute timelock constraints /// that are not satisfied at the given `n` (`n OP_CHECKLOCKTIMEVERIFY`). - pub fn at_lock_time(mut self, n: u32) -> Policy { + pub fn at_lock_time(mut self, n: LockTime) -> Policy { self = match self { Policy::After(t) => { - if !timelock::absolute_timelocks_are_same_unit(t, n) { + if !t.is_same_unit(n) { Policy::Unsatisfiable } else if t > n { Policy::Unsatisfiable @@ -639,12 +653,21 @@ impl Policy { /// in general this appears to require Gröbner basis techniques that are not /// implemented. pub fn sorted(self) -> Policy { + use Policy::*; + match self { Policy::Threshold(k, subs) => { let mut new_subs: Vec<_> = subs.into_iter().map(Policy::sorted).collect(); - new_subs.sort(); + new_subs.sort_by(|a, b| { + // partial_cmp on the enum should give an ordering for any a/b that are + // different as well as for any a/b that are the same except After/After. + match (a, b) { + (After(a), After(b)) => a.to_consensus_u32().cmp(&b.to_consensus_u32()), + (a, b) => a.partial_cmp(b).unwrap_or(Ordering::Equal), + } + }); Policy::Threshold(k, new_subs) - } + } x => x, } } @@ -795,42 +818,33 @@ mod tests { // Block height 1000. let policy = StringPolicy::from_str("after(1000)").unwrap(); - assert_eq!(policy, Policy::After(1000)); + assert_eq!(policy, Policy::after(1000)); assert_eq!(policy.absolute_timelocks(), vec![1000]); assert_eq!(policy.relative_timelocks(), vec![]); - assert_eq!(policy.clone().at_lock_time(0), Policy::Unsatisfiable); - assert_eq!(policy.clone().at_lock_time(999), Policy::Unsatisfiable); - assert_eq!(policy.clone().at_lock_time(1000), policy.clone()); - assert_eq!(policy.clone().at_lock_time(10000), policy.clone()); + assert_eq!(policy.clone().at_lock_time(LockTime::from_consensus(0)), Policy::Unsatisfiable); + assert_eq!(policy.clone().at_lock_time(LockTime::from_consensus(999)), Policy::Unsatisfiable); + assert_eq!(policy.clone().at_lock_time(LockTime::from_consensus(1000)), policy.clone()); + assert_eq!(policy.clone().at_lock_time(LockTime::from_consensus(10000)), policy.clone()); // Pass a UNIX timestamp to at_lock_time while policy uses a block height. - assert_eq!( - policy.clone().at_lock_time(500_000_001), - Policy::Unsatisfiable - ); + assert_eq!(policy.clone().at_lock_time(LockTime::from_consensus(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, 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_lock_time while policy uses a UNIX timestapm. - assert_eq!(policy.clone().at_lock_time(0), Policy::Unsatisfiable); - assert_eq!(policy.clone().at_lock_time(999), Policy::Unsatisfiable); - assert_eq!(policy.clone().at_lock_time(1000), Policy::Unsatisfiable); - assert_eq!(policy.clone().at_lock_time(10000), Policy::Unsatisfiable); + assert_eq!(policy.clone().at_lock_time(LockTime::from_consensus(0)), Policy::Unsatisfiable); + assert_eq!(policy.clone().at_lock_time(LockTime::from_consensus(999)), Policy::Unsatisfiable); + assert_eq!(policy.clone().at_lock_time(LockTime::from_consensus(1000)), Policy::Unsatisfiable); + assert_eq!(policy.clone().at_lock_time(LockTime::from_consensus(10000)), Policy::Unsatisfiable); // And now pass a UNIX timestamp to at_lock_time while policy also uses a timestamp. - assert_eq!( - policy.clone().at_lock_time(500_000_000), - Policy::Unsatisfiable - ); - assert_eq!( - policy.clone().at_lock_time(500_000_001), - Policy::Unsatisfiable - ); - assert_eq!(policy.clone().at_lock_time(500_000_010), policy.clone()); - assert_eq!(policy.clone().at_lock_time(500_000_012), policy.clone()); + assert_eq!(policy.clone().at_lock_time(LockTime::from_consensus(500_000_000)), Policy::Unsatisfiable); + assert_eq!(policy.clone().at_lock_time(LockTime::from_consensus(500_000_001)), Policy::Unsatisfiable); + assert_eq!(policy.clone().at_lock_time(LockTime::from_consensus(500_000_010)), policy.clone()); + assert_eq!(policy.clone().at_lock_time(LockTime::from_consensus(500_000_012)), policy.clone()); assert_eq!(policy.n_keys(), 0); assert_eq!(policy.minimum_n_keys(), Some(0)); } diff --git a/src/psbt/mod.rs b/src/psbt/mod.rs index 50a7144f7..9b08ba962 100644 --- a/src/psbt/mod.rs +++ b/src/psbt/mod.rs @@ -29,12 +29,12 @@ use bitcoin::secp256k1::{self, Secp256k1, VerifyOnly}; use bitcoin::util::psbt::{self, PartiallySignedTransaction as Psbt}; use bitcoin::util::sighash::SighashCache; use bitcoin::util::taproot::{self, ControlBlock, LeafVersion, TapLeafHash}; -use bitcoin::{self, EcdsaSighashType, SchnorrSighashType, Script}; +use bitcoin::{self, EcdsaSighashType, LockTime, SchnorrSighashType, Script}; use crate::miniscript::iter::PkPkh; use crate::miniscript::limits::SEQUENCE_LOCKTIME_DISABLE_FLAG; -use crate::miniscript::satisfy::{After, Older}; use crate::prelude::*; +use crate::miniscript::satisfy::Older; use crate::{ descriptor, interpreter, Descriptor, DescriptorPublicKey, MiniscriptKey, PkTranslator, Preimage32, Satisfier, ToPublicKey, TranslatePk, @@ -334,16 +334,19 @@ impl<'psbt, Pk: MiniscriptKey + ToPublicKey> Satisfier for PsbtInputSatisfie .map(|(pk, sig)| (*pk, *sig)) } - fn check_after(&self, n: u32) -> bool { - let locktime = self.psbt.unsigned_tx.lock_time; - let seq = self.psbt.unsigned_tx.input[self.index].sequence; + fn check_after(&self, n: LockTime) -> bool { + use LockTime::*; - // https://github.com/bitcoin/bips/blob/master/bip-0065.mediawiki - // fail if TxIn is finalized - if seq == 0xffffffff { - false - } else { - >::check_after(&After(locktime), n) + if !self.psbt.unsigned_tx.input[self.index].enables_lock_time() { + return false; + } + + let lock_time = LockTime::from(self.psbt.unsigned_tx.lock_time); + + match (n, lock_time) { + (Blocks(n), Blocks(lock_time)) => n <= lock_time, + (Seconds(n), Seconds(lock_time)) => n <= lock_time, + _ => false, // Not the same units. } } @@ -1257,6 +1260,7 @@ mod tests { use bitcoin::secp256k1::PublicKey; use bitcoin::util::bip32::{DerivationPath, ExtendedPubKey}; use bitcoin::{OutPoint, TxIn, TxOut, XOnlyPublicKey}; + use bitcoin::blockdata::locktime; use super::*; use crate::Miniscript; @@ -1443,7 +1447,7 @@ mod tests { let mut non_witness_utxo = bitcoin::Transaction { version: 1, - lock_time: 0, + lock_time: locktime::ZERO, input: vec![], output: vec![TxOut { value: 1_000, @@ -1456,7 +1460,7 @@ mod tests { let tx = bitcoin::Transaction { version: 1, - lock_time: 0, + lock_time: locktime::ZERO, input: vec![TxIn { previous_output: OutPoint { txid: non_witness_utxo.txid(), diff --git a/src/timelock.rs b/src/timelock.rs deleted file mode 100644 index a28e211e8..000000000 --- a/src/timelock.rs +++ /dev/null @@ -1,21 +0,0 @@ -//! 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 -} diff --git a/tests/test_cpp.rs b/tests/test_cpp.rs index a6e7fd1a3..37bd4c460 100644 --- a/tests/test_cpp.rs +++ b/tests/test_cpp.rs @@ -13,7 +13,7 @@ use bitcoin::hashes::{sha256d, Hash}; use bitcoin::secp256k1::{self, Secp256k1}; use bitcoin::util::psbt; use bitcoin::util::psbt::PartiallySignedTransaction as Psbt; -use bitcoin::{self, Amount, OutPoint, Transaction, TxIn, TxOut, Txid}; +use bitcoin::{self, Amount, OutPoint, Transaction, TxIn, TxOut, Txid, LockTime}; use bitcoind::bitcoincore_rpc::{json, Client, RpcApi}; use miniscript::miniscript::iter; use miniscript::psbt::PsbtExt; @@ -94,6 +94,8 @@ pub fn test_from_cpp_ms(cl: &Client, testdata: &TestData) { None, None, None, + None, + None, ) .unwrap(); txids.push(txid); @@ -110,7 +112,7 @@ pub fn test_from_cpp_ms(cl: &Client, testdata: &TestData) { let mut psbt = Psbt { unsigned_tx: Transaction { version: 2, - lock_time: 1_603_866_330, // time at 10/28/2020 @ 6:25am (UTC) + lock_time: LockTime::from_consensus(1_603_866_330), // time at 10/28/2020 @ 6:25am (UTC) input: vec![], output: vec![], }, @@ -122,7 +124,7 @@ pub fn test_from_cpp_ms(cl: &Client, testdata: &TestData) { outputs: vec![], }; // figure out the outpoint from the txid - let (outpoint, witness_utxo) = get_vout(&cl, txid, btc(1.0).as_sat()); + let (outpoint, witness_utxo) = get_vout(&cl, txid, btc(1.0).to_sat()); let mut txin = TxIn::default(); txin.previous_output = outpoint; // set the sequence to a non-final number for the locktime transactions to be @@ -169,7 +171,7 @@ pub fn test_from_cpp_ms(cl: &Client, testdata: &TestData) { }) .collect(); // Get the required sighash message - let amt = btc(1).as_sat(); + let amt = btc(1).to_sat(); let mut sighash_cache = bitcoin::util::sighash::SighashCache::new(&psbts[i].unsigned_tx); let sighash_ty = bitcoin::EcdsaSighashType::All; let sighash = sighash_cache diff --git a/tests/test_desc.rs b/tests/test_desc.rs index 425b1da6d..9f69b27a9 100644 --- a/tests/test_desc.rs +++ b/tests/test_desc.rs @@ -15,7 +15,7 @@ use bitcoin::util::sighash::SighashCache; use bitcoin::util::taproot::{LeafVersion, TapLeafHash}; use bitcoin::util::{psbt, sighash}; use bitcoin::{ - self, secp256k1, Amount, OutPoint, SchnorrSig, Script, Transaction, TxIn, TxOut, Txid, + self, secp256k1, Amount, OutPoint, SchnorrSig, Script, Transaction, TxIn, TxOut, Txid, LockTime }; use bitcoind::bitcoincore_rpc::{json, Client, RpcApi}; use miniscript::miniscript::iter; @@ -92,7 +92,7 @@ pub fn test_desc_satisfy( // Next send some btc to each address corresponding to the miniscript let txid = cl - .send_to_address(&desc_address, btc(1), None, None, None, None, None, None) + .send_to_address(&desc_address, btc(1), None, None, None, None, None, None, None, None) .unwrap(); // Wait for the funds to mature. let blocks = cl @@ -104,7 +104,7 @@ pub fn test_desc_satisfy( let mut psbt = Psbt { unsigned_tx: Transaction { version: 2, - lock_time: 1_603_866_330, // time at 10/28/2020 @ 6:25am (UTC) + lock_time: LockTime::from_consensus(1_603_866_330), // time at 10/28/2020 @ 6:25am (UTC) input: vec![], output: vec![], }, @@ -117,7 +117,7 @@ pub fn test_desc_satisfy( }; // figure out the outpoint from the txid let (outpoint, witness_utxo) = - get_vout(&cl, txid, btc(1.0).as_sat(), derived_desc.script_pubkey()); + get_vout(&cl, txid, btc(1.0).to_sat(), derived_desc.script_pubkey()); let mut txin = TxIn::default(); txin.previous_output = outpoint; // set the sequence to a non-final number for the locktime transactions to be