Skip to content

Commit c0974af

Browse files
heap-coderheap-coder
committed
policy: use RelLockTime in semantic and concrete policies
This changes the signature of `at_age` to take a `relative::LockTime` to be symmetric with `at_lock_time`. But because `relative::LockTime` has no constructors this is kinda annoying to use. See rust-bitcoin/rust-bitcoin#2547 Anyway for now you can just construct a RelLockTime and tack .into() onto it, which is fine as a workaround. We may want to update docs for it.
1 parent d0cad43 commit c0974af

File tree

5 files changed

+90
-101
lines changed

5 files changed

+90
-101
lines changed

‎src/miniscript/mod.rs‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -698,7 +698,6 @@ mod tests {
698698
use bitcoin::hashes::{hash160, sha256, Hash};
699699
use bitcoin::secp256k1::XOnlyPublicKey;
700700
use bitcoin::taproot::TapLeafHash;
701-
use bitcoin::Sequence;
702701
use sync::Arc;
703702

704703
use super::{Miniscript, ScriptContext, Segwitv0, Tap};
@@ -707,7 +706,7 @@ mod tests {
707706
use crate::policy::Liftable;
708707
use crate::prelude::*;
709708
use crate::test_utils::{StrKeyTranslator, StrXOnlyKeyTranslator};
710-
use crate::{hex_script, ExtParams, Satisfier, ToPublicKey, TranslatePk};
709+
use crate::{hex_script, ExtParams, RelLockTime, Satisfier, ToPublicKey, TranslatePk};
711710

712711
type Segwitv0Script = Miniscript<bitcoin::PublicKey, Segwitv0>;
713712
type Tapscript = Miniscript<bitcoin::secp256k1::XOnlyPublicKey, Tap>;
@@ -1084,13 +1083,13 @@ mod tests {
10841083
let mut abs = miniscript.lift().unwrap();
10851084
assert_eq!(abs.n_keys(), 5);
10861085
assert_eq!(abs.minimum_n_keys(), Some(2));
1087-
abs = abs.at_age(Sequence::from_height(10000));
1086+
abs = abs.at_age(RelLockTime::from_height(10000).into());
10881087
assert_eq!(abs.n_keys(), 5);
10891088
assert_eq!(abs.minimum_n_keys(), Some(2));
1090-
abs = abs.at_age(Sequence::from_height(9999));
1089+
abs = abs.at_age(RelLockTime::from_height(9999).into());
10911090
assert_eq!(abs.n_keys(), 3);
10921091
assert_eq!(abs.minimum_n_keys(), Some(3));
1093-
abs = abs.at_age(Sequence::ZERO);
1092+
abs = abs.at_age(RelLockTime::ZERO.into());
10941093
assert_eq!(abs.n_keys(), 3);
10951094
assert_eq!(abs.minimum_n_keys(), Some(3));
10961095

‎src/policy/compiler.rs‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -871,7 +871,7 @@ where
871871
insert_wrap!(AstElemExt::terminal(Terminal::PkK(pk.clone())));
872872
}
873873
Concrete::After(n) => insert_wrap!(AstElemExt::terminal(Terminal::After(n))),
874-
Concrete::Older(n) => insert_wrap!(AstElemExt::terminal(Terminal::Older(n))),
874+
Concrete::Older(n) => insert_wrap!(AstElemExt::terminal(Terminal::Older(n.into()))),
875875
Concrete::Sha256(ref hash) => {
876876
insert_wrap!(AstElemExt::terminal(Terminal::Sha256(hash.clone())))
877877
}
@@ -1254,7 +1254,7 @@ mod tests {
12541254
use super::*;
12551255
use crate::miniscript::{Legacy, Segwitv0, Tap};
12561256
use crate::policy::Liftable;
1257-
use crate::{script_num_size, AbsLockTime, ToPublicKey};
1257+
use crate::{script_num_size, AbsLockTime, RelLockTime, ToPublicKey};
12581258

12591259
type SPolicy = Concrete<String>;
12601260
type BPolicy = Concrete<bitcoin::PublicKey>;
@@ -1406,7 +1406,7 @@ mod tests {
14061406
(
14071407
1,
14081408
Arc::new(Concrete::And(vec![
1409-
Arc::new(Concrete::Older(Sequence::from_height(10000))),
1409+
Arc::new(Concrete::Older(RelLockTime::from_height(10000))),
14101410
Arc::new(Concrete::Threshold(
14111411
2,
14121412
key_pol[5..8].iter().map(|p| (p.clone()).into()).collect(),
@@ -1436,13 +1436,13 @@ mod tests {
14361436
let mut abs = policy.lift().unwrap();
14371437
assert_eq!(abs.n_keys(), 8);
14381438
assert_eq!(abs.minimum_n_keys(), Some(2));
1439-
abs = abs.at_age(Sequence::from_height(10000));
1439+
abs = abs.at_age(RelLockTime::from_height(10000).into());
14401440
assert_eq!(abs.n_keys(), 8);
14411441
assert_eq!(abs.minimum_n_keys(), Some(2));
1442-
abs = abs.at_age(Sequence::from_height(9999));
1442+
abs = abs.at_age(RelLockTime::from_height(9999).into());
14431443
assert_eq!(abs.n_keys(), 5);
14441444
assert_eq!(abs.minimum_n_keys(), Some(3));
1445-
abs = abs.at_age(Sequence::ZERO);
1445+
abs = abs.at_age(RelLockTime::ZERO.into());
14461446
assert_eq!(abs.n_keys(), 5);
14471447
assert_eq!(abs.minimum_n_keys(), Some(3));
14481448

@@ -1467,15 +1467,16 @@ mod tests {
14671467
assert!(ms.satisfy(no_sat).is_err());
14681468
assert!(ms.satisfy(&left_sat).is_ok());
14691469
assert!(ms
1470-
.satisfy((&right_sat, Sequence::from_height(10001)))
1470+
.satisfy((&right_sat, RelLockTime::from_height(10001)))
14711471
.is_ok());
14721472
//timelock not met
14731473
assert!(ms
1474-
.satisfy((&right_sat, Sequence::from_height(9999)))
1474+
.satisfy((&right_sat, RelLockTime::from_height(9999)))
14751475
.is_err());
14761476

14771477
assert_eq!(
1478-
ms.satisfy((left_sat, Sequence::from_height(9999))).unwrap(),
1478+
ms.satisfy((left_sat, RelLockTime::from_height(9999)))
1479+
.unwrap(),
14791480
vec![
14801481
// sat for left branch
14811482
vec![],
@@ -1486,7 +1487,7 @@ mod tests {
14861487
);
14871488

14881489
assert_eq!(
1489-
ms.satisfy((right_sat, Sequence::from_height(10000)))
1490+
ms.satisfy((right_sat, RelLockTime::from_height(10000)))
14901491
.unwrap(),
14911492
vec![
14921493
// sat for right branch

‎src/policy/concrete.rs‎

Lines changed: 11 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use core::{fmt, str};
77
#[cfg(feature = "std")]
88
use std::error;
99

10-
use bitcoin::{absolute, Sequence};
10+
use bitcoin::absolute;
1111
#[cfg(feature = "compiler")]
1212
use {
1313
crate::descriptor::TapTree,
@@ -29,7 +29,9 @@ use crate::prelude::*;
2929
use crate::sync::Arc;
3030
#[cfg(all(doc, not(feature = "compiler")))]
3131
use crate::Descriptor;
32-
use crate::{errstr, AbsLockTime, Error, ForEachKey, FromStrKey, MiniscriptKey, Translator};
32+
use crate::{
33+
errstr, AbsLockTime, Error, ForEachKey, FromStrKey, MiniscriptKey, RelLockTime, Translator,
34+
};
3335

3436
/// Maximum TapLeafs allowed in a compiled TapTree
3537
#[cfg(feature = "compiler")]
@@ -52,7 +54,7 @@ pub enum Policy<Pk: MiniscriptKey> {
5254
/// An absolute locktime restriction.
5355
After(AbsLockTime),
5456
/// A relative locktime restriction.
55-
Older(Sequence),
57+
Older(RelLockTime),
5658
/// A SHA256 whose preimage must be provided to satisfy the descriptor.
5759
Sha256(Pk::Sha256),
5860
/// A SHA256d whose preimage must be provided to satisfy the descriptor.
@@ -70,15 +72,6 @@ pub enum Policy<Pk: MiniscriptKey> {
7072
Threshold(usize, Vec<Arc<Policy<Pk>>>),
7173
}
7274

73-
impl<Pk> Policy<Pk>
74-
where
75-
Pk: MiniscriptKey,
76-
{
77-
/// Construct a `Policy::Older` from `n`. Helper function equivalent to
78-
/// `Policy::Older(Sequence::from_consensus(n))`.
79-
pub fn older(n: u32) -> Policy<Pk> { Policy::Older(Sequence::from_consensus(n)) }
80-
}
81-
8275
/// Detailed error type for concrete policies.
8376
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
8477
pub enum PolicyError {
@@ -88,10 +81,6 @@ pub enum PolicyError {
8881
NonBinaryArgOr,
8982
/// `Thresh` fragment can only have `1<=k<=n`.
9083
IncorrectThresh,
91-
/// `older` or `after` fragment can only have `n = 0`.
92-
ZeroTime,
93-
/// `after` fragment can only have `n < 2^31`.
94-
TimeTooFar,
9584
/// Semantic Policy Error: `And` `Or` fragments must take args: `k > 1`.
9685
InsufficientArgsforAnd,
9786
/// Semantic policy error: `And` `Or` fragments must take args: `k > 1`.
@@ -129,10 +118,6 @@ impl fmt::Display for PolicyError {
129118
PolicyError::IncorrectThresh => {
130119
f.write_str("Threshold k must be greater than 0 and less than or equal to n 0<k<=n")
131120
}
132-
PolicyError::TimeTooFar => {
133-
f.write_str("Relative/Absolute time must be less than 2^31; n < 2^31")
134-
}
135-
PolicyError::ZeroTime => f.write_str("Time must be greater than 0; n > 0"),
136121
PolicyError::InsufficientArgsforAnd => {
137122
f.write_str("Semantic Policy 'And' fragment must have at least 2 args ")
138123
}
@@ -159,8 +144,6 @@ impl error::Error for PolicyError {
159144
NonBinaryArgAnd
160145
| NonBinaryArgOr
161146
| IncorrectThresh
162-
| ZeroTime
163-
| TimeTooFar
164147
| InsufficientArgsforAnd
165148
| InsufficientArgsforOr
166149
| EntailmentMaxTerminals
@@ -751,13 +734,7 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
751734
for policy in self.pre_order_iter() {
752735
match *policy {
753736
After(_) => {}
754-
Older(n) => {
755-
if n == Sequence::ZERO {
756-
return Err(PolicyError::ZeroTime);
757-
} else if n.to_consensus_u32() > 2u32.pow(31) {
758-
return Err(PolicyError::TimeTooFar);
759-
}
760-
}
737+
Older(_) => {}
761738
And(ref subs) => {
762739
if subs.len() != 2 {
763740
return Err(PolicyError::NonBinaryArgAnd);
@@ -971,15 +948,11 @@ impl<Pk: FromStrKey> Policy<Pk> {
971948
.and_then(|x| AbsLockTime::from_consensus(x).map_err(Error::AbsoluteLockTime))
972949
.map(Policy::After)
973950
}),
974-
("older", 1) => {
975-
let num = expression::terminal(&top.args[0], expression::parse_num)?;
976-
if num > 2u32.pow(31) {
977-
return Err(Error::PolicyError(PolicyError::TimeTooFar));
978-
} else if num == 0 {
979-
return Err(Error::PolicyError(PolicyError::ZeroTime));
980-
}
981-
Ok(Policy::older(num))
982-
}
951+
("older", 1) => expression::terminal(&top.args[0], |x| {
952+
expression::parse_num(x)
953+
.and_then(|x| RelLockTime::from_consensus(x).map_err(Error::RelativeLockTime))
954+
.map(Policy::Older)
955+
}),
983956
("sha256", 1) => expression::terminal(&top.args[0], |x| {
984957
<Pk::Sha256 as core::str::FromStr>::from_str(x).map(Policy::Sha256)
985958
}),

‎src/policy/mod.rs‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,9 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Liftable<Pk> for Terminal<Pk, Ctx> {
123123
return Err(Error::LiftError(LiftError::RawDescriptorLift))
124124
}
125125
Terminal::After(t) => Semantic::After(t),
126-
Terminal::Older(t) => Semantic::Older(t),
126+
Terminal::Older(t) => Semantic::Older(
127+
<crate::RelLockTime as core::convert::TryFrom<_>>::try_from(t).unwrap(),
128+
), // unwrap to be removed in future commit
127129
Terminal::Sha256(ref h) => Semantic::Sha256(h.clone()),
128130
Terminal::Hash256(ref h) => Semantic::Hash256(h.clone()),
129131
Terminal::Ripemd160(ref h) => Semantic::Ripemd160(h.clone()),
@@ -241,13 +243,12 @@ impl<Pk: MiniscriptKey> Liftable<Pk> for Arc<Concrete<Pk>> {
241243
mod tests {
242244
use core::str::FromStr;
243245

244-
use bitcoin::Sequence;
245-
246246
use super::*;
247247
#[cfg(feature = "compiler")]
248248
use crate::descriptor::Tr;
249249
use crate::miniscript::context::Segwitv0;
250250
use crate::prelude::*;
251+
use crate::RelLockTime;
251252
#[cfg(feature = "compiler")]
252253
use crate::{descriptor::TapTree, Tap};
253254

@@ -323,7 +324,7 @@ mod tests {
323324
ConcretePol::from_str("or(pk())").unwrap_err().to_string(),
324325
"Or policy fragment must take 2 arguments"
325326
);
326-
// this weird "unexpected" wrapping of the error will go away in a later PR
327+
// these weird "unexpected" wrapping of errors will go away in a later PR
327328
// which rewrites the expression parser
328329
assert_eq!(
329330
ConcretePol::from_str("thresh(3,after(0),pk(),pk())")
@@ -336,7 +337,7 @@ mod tests {
336337
ConcretePol::from_str("thresh(2,older(2147483650),pk(),pk())")
337338
.unwrap_err()
338339
.to_string(),
339-
"Relative/Absolute time must be less than 2^31; n < 2^31"
340+
"unexpected «locktime value 2147483650 is not a valid BIP68 relative locktime»"
340341
);
341342
}
342343

@@ -370,7 +371,7 @@ mod tests {
370371
2,
371372
vec![
372373
Arc::new(Semantic::Key(key_a)),
373-
Arc::new(Semantic::Older(Sequence::from_height(42)))
374+
Arc::new(Semantic::Older(RelLockTime::from_height(42)))
374375
]
375376
)),
376377
Arc::new(Semantic::Key(key_b))

0 commit comments

Comments
 (0)