miniscript: use RelLockTime in Terminal

This gets rid of all the new conversions and unwraps :)

Author: heap-coder

src/interpreter/mod.rs

@@ -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.sequence);
+                    let res = self.stack.evaluate_older(&(*n).into(), self.sequence);
                     if res.is_some() {
                         return res;
                     }

src/miniscript/astelem.rs

@@ -11,16 +11,16 @@ use core::fmt;
 use core::str::FromStr;
 
 use bitcoin::hashes::{hash160, Hash};
-use bitcoin::{absolute, opcodes, script, Sequence};
+use bitcoin::{absolute, opcodes, script};
 use sync::Arc;
 
 use crate::miniscript::context::SigType;
 use crate::miniscript::{types, ScriptContext};
 use crate::prelude::*;
 use crate::util::MsKeyBuilder;
 use crate::{
-    errstr, expression, AbsLockTime, Error, FromStrKey, Miniscript, MiniscriptKey, Terminal,
-    ToPublicKey,
+    errstr, expression, AbsLockTime, Error, FromStrKey, Miniscript, MiniscriptKey, RelLockTime,
+    Terminal, ToPublicKey,
 };
 
 impl<Pk: MiniscriptKey, Ctx: ScriptContext> Terminal<Pk, Ctx> {
@@ -261,7 +261,9 @@ impl<Pk: FromStrKey, Ctx: ScriptContext> crate::expression::FromTree for Termina
                     .map(Terminal::After)
             }),
             ("older", 1) => expression::terminal(&top.args[0], |x| {
-                expression::parse_num(x).map(|x| Terminal::Older(Sequence::from_consensus(x)))
+                expression::parse_num(x)
+                    .and_then(|x| RelLockTime::from_consensus(x).map_err(Error::RelativeLockTime))
+                    .map(Terminal::Older)
             }),
             ("sha256", 1) => expression::terminal(&top.args[0], |x| {
                 Pk::Sha256::from_str(x).map(Terminal::Sha256)

src/miniscript/decode.rs

@@ -11,7 +11,7 @@ use core::marker::PhantomData;
 use std::error;
 
 use bitcoin::hashes::{hash160, ripemd160, sha256, Hash};
-use bitcoin::{Sequence, Weight};
+use bitcoin::Weight;
 use sync::Arc;
 
 use crate::miniscript::lex::{Token as Tk, TokenIter};
@@ -22,7 +22,7 @@ use crate::miniscript::ScriptContext;
 use crate::prelude::*;
 #[cfg(doc)]
 use crate::Descriptor;
-use crate::{hash256, AbsLockTime, Error, Miniscript, MiniscriptKey, ToPublicKey};
+use crate::{hash256, AbsLockTime, Error, Miniscript, MiniscriptKey, RelLockTime, ToPublicKey};
 
 /// Trait for parsing keys from byte slices
 pub trait ParseableKey: Sized + ToPublicKey + private::Sealed {
@@ -137,7 +137,7 @@ pub enum Terminal<Pk: MiniscriptKey, Ctx: ScriptContext> {
     /// `n CHECKLOCKTIMEVERIFY`
     After(AbsLockTime),
     /// `n CHECKSEQUENCEVERIFY`
-    Older(Sequence),
+    Older(RelLockTime),
     // hashlocks
     /// `SIZE 32 EQUALVERIFY SHA256 <hash> EQUAL`
     Sha256(Pk::Sha256),
@@ -357,7 +357,7 @@ pub fn parse<Ctx: ScriptContext>(
                     },
                     // timelocks
                     Tk::CheckSequenceVerify, Tk::Num(n)
-                        => term.reduce0(Terminal::Older(Sequence::from_consensus(n)))?,
+                        => term.reduce0(Terminal::Older(RelLockTime::from_consensus(n).map_err(Error::RelativeLockTime)?))?,
                     Tk::CheckLockTimeVerify, Tk::Num(n)
                         => term.reduce0(Terminal::After(AbsLockTime::from_consensus(n).map_err(Error::AbsoluteLockTime)?))?,
                     // hashlocks

src/miniscript/satisfy.rs

@@ -1285,8 +1285,6 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> {
                 Satisfaction { stack, has_sig: false, relative_timelock: None, absolute_timelock }
             }
             Terminal::Older(t) => {
-                // unwrap to be removed in a later commit
-                let t = <RelLockTime as core::convert::TryFrom<_>>::try_from(t).unwrap();
                 let (stack, relative_timelock) = if stfr.check_older(t.into()) {
                     (Witness::empty(), Some(t))
                 } else if root_has_sig {

src/miniscript/types/extra_props.rs

@@ -6,12 +6,10 @@
 use core::cmp;
 use core::iter::once;
 
-use bitcoin::Sequence;
-
 use super::{Error, ErrorKind, ScriptContext};
 use crate::miniscript::context::SigType;
 use crate::prelude::*;
-use crate::{script_num_size, AbsLockTime, MiniscriptKey, Terminal};
+use crate::{script_num_size, AbsLockTime, MiniscriptKey, RelLockTime, Terminal};
 
 /// Timelock information for satisfaction of a fragment.
 #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Debug, Default, Hash)]
@@ -365,7 +363,7 @@ impl ExtData {
     }
 
     /// Extra properties for the `older` fragment.
-    pub fn older(t: Sequence) -> Self {
+    pub fn older(t: RelLockTime) -> Self {
         ExtData {
             pk_cost: script_num_size(t.to_consensus_u32() as usize) + 1,
             has_free_verify: false,
@@ -924,15 +922,7 @@ impl ExtData {
                 }
             }
             Terminal::After(t) => Self::after(t),
-            Terminal::Older(t) => {
-                if t == Sequence::ZERO || !t.is_relative_lock_time() {
-                    return Err(Error {
-                        fragment_string: fragment.to_string(),
-                        error: ErrorKind::InvalidTime,
-                    });
-                }
-                Self::older(t)
-            }
+            Terminal::Older(t) => Self::older(t),
             Terminal::Sha256(..) => Self::sha256(),
             Terminal::Hash256(..) => Self::hash256(),
             Terminal::Ripemd160(..) => Self::ripemd160(),

src/miniscript/types/mod.rs

@@ -14,8 +14,6 @@ use core::fmt;
 #[cfg(feature = "std")]
 use std::error;
 
-use bitcoin::Sequence;
-
 pub use self::correctness::{Base, Correctness, Input};
 pub use self::extra_props::ExtData;
 pub use self::malleability::{Dissat, Malleability};
@@ -25,8 +23,6 @@ use crate::{MiniscriptKey, Terminal};
 /// Detailed type of a typechecker error
 #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
 pub enum ErrorKind {
-    /// Relative or absolute timelock had an invalid time value (either 0, or >=0x80000000)
-    InvalidTime,
     /// Passed a `z` argument to a `d` wrapper when `z` was expected
     NonZeroDupIf,
     /// Multisignature or threshold policy had a `k` value of 0
@@ -79,11 +75,6 @@ pub struct Error {
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self.error {
-            ErrorKind::InvalidTime => write!(
-                f,
-                "fragment «{}» represents a timelock which value is invalid (time must be in [1; 0x80000000])",
-                self.fragment_string,
-            ),
             ErrorKind::NonZeroDupIf => write!(
                 f,
                 "fragment «{}» represents needs to be `z`, needs to consume zero elements from the stack",
@@ -479,15 +470,7 @@ impl Type {
                 }
             }
             Terminal::After(_) => Ok(Self::time()),
-            Terminal::Older(t) => {
-                if t == Sequence::ZERO || !t.is_relative_lock_time() {
-                    return Err(Error {
-                        fragment_string: fragment.to_string(),
-                        error: ErrorKind::InvalidTime,
-                    });
-                }
-                Ok(Self::time())
-            }
+            Terminal::Older(_) => Ok(Self::time()),
             Terminal::Sha256(..) => Ok(Self::hash()),
             Terminal::Hash256(..) => Ok(Self::hash()),
             Terminal::Ripemd160(..) => Ok(Self::hash()),

src/policy/compiler.rs

@@ -9,7 +9,6 @@ use core::{cmp, f64, fmt, hash, mem};
 #[cfg(feature = "std")]
 use std::error;
 
-use bitcoin::Sequence;
 use sync::Arc;
 
 use crate::miniscript::context::SigType;
@@ -447,15 +446,7 @@ impl CompilerExtData {
                 }
             }
             Terminal::After(_) => Ok(Self::time()),
-            Terminal::Older(t) => {
-                if t == Sequence::ZERO || !t.is_relative_lock_time() {
-                    return Err(types::Error {
-                        fragment_string: fragment.to_string(),
-                        error: types::ErrorKind::InvalidTime,
-                    });
-                }
-                Ok(Self::time())
-            }
+            Terminal::Older(_) => Ok(Self::time()),
             Terminal::Sha256(..) => Ok(Self::hash()),
             Terminal::Hash256(..) => Ok(Self::hash()),
             Terminal::Ripemd160(..) => Ok(Self::hash()),
@@ -871,7 +862,7 @@ where
             insert_wrap!(AstElemExt::terminal(Terminal::PkK(pk.clone())));
         }
         Concrete::After(n) => insert_wrap!(AstElemExt::terminal(Terminal::After(n))),
-        Concrete::Older(n) => insert_wrap!(AstElemExt::terminal(Terminal::Older(n.into()))),
+        Concrete::Older(n) => insert_wrap!(AstElemExt::terminal(Terminal::Older(n))),
         Concrete::Sha256(ref hash) => {
             insert_wrap!(AstElemExt::terminal(Terminal::Sha256(hash.clone())))
         }

src/policy/mod.rs

@@ -123,9 +123,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Liftable<Pk> for Terminal<Pk, Ctx> {
                 return Err(Error::LiftError(LiftError::RawDescriptorLift))
             }
             Terminal::After(t) => Semantic::After(t),
-            Terminal::Older(t) => Semantic::Older(
-                <crate::RelLockTime as core::convert::TryFrom<_>>::try_from(t).unwrap(),
-            ), // unwrap to be removed in future commit
+            Terminal::Older(t) => Semantic::Older(t),
             Terminal::Sha256(ref h) => Semantic::Sha256(h.clone()),
             Terminal::Hash256(ref h) => Semantic::Hash256(h.clone()),
             Terminal::Ripemd160(ref h) => Semantic::Ripemd160(h.clone()),