Skip to content

Commit

Permalink
interpreter: use relative::LockTime for constraints from Miniscript
Browse files Browse the repository at this point in the history
In general, the type from the transaction should be `Sequence` while the
type we are comparing to should be `RelLockTime` or
`relative::LockTime`. Having different types here should help us avoid
getting these comparisons backward.
  • Loading branch information
apoelstra committed Mar 6, 2024
1 parent 66a0cec commit 9b3d45d
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 21 deletions.
14 changes: 10 additions & 4 deletions src/interpreter/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use bitcoin::hashes::hash160;
use bitcoin::hex::DisplayHex;
#[cfg(not(test))] // https://github.com/rust-lang/rust/issues/121684
use bitcoin::secp256k1;
use bitcoin::taproot;
use bitcoin::{absolute, relative, taproot};

use super::BitcoinKey;
use crate::prelude::*;
Expand All @@ -18,9 +18,9 @@ use crate::prelude::*;
#[derive(Debug)]
pub enum Error {
/// Could not satisfy, absolute locktime not met
AbsoluteLocktimeNotMet(u32),
AbsoluteLocktimeNotMet(absolute::LockTime),
/// Could not satisfy, lock time values are different units
AbsoluteLocktimeComparisonInvalid(u32, u32),
AbsoluteLocktimeComparisonInvalid(absolute::LockTime, absolute::LockTime),
/// 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
Expand Down Expand Up @@ -85,7 +85,9 @@ pub enum Error {
/// Parse Error while parsing a `stack::Element::Push` as a XOnlyPublicKey (32 bytes)
XOnlyPublicKeyParseError,
/// Could not satisfy, relative locktime not met
RelativeLocktimeNotMet(u32),
RelativeLocktimeNotMet(relative::LockTime),
/// Could not satisfy, the sequence number on the tx input had the disable flag set.
RelativeLocktimeDisabled(relative::LockTime),
/// Forward-secp related errors
Secp(secp256k1::Error),
/// Miniscript requires the entire top level script to be satisfied.
Expand Down Expand Up @@ -162,6 +164,9 @@ impl fmt::Display for Error {
Error::RelativeLocktimeNotMet(n) => {
write!(f, "required relative locktime CSV of {} blocks, not met", n)
}
Error::RelativeLocktimeDisabled(n) => {
write!(f, "required relative locktime CSV of {} blocks, but tx sequence number has disable-flag set", n)
}
Error::ScriptSatisfactionError => f.write_str("Top level script must be satisfied"),
Error::Secp(ref e) => fmt::Display::fmt(e, f),
Error::SchnorrSig(ref s) => write!(f, "Schnorr sig error: {}", s),
Expand Down Expand Up @@ -213,6 +218,7 @@ impl error::Error for Error {
| PkEvaluationError(_)
| PkHashVerifyFail(_)
| RelativeLocktimeNotMet(_)
| RelativeLocktimeDisabled(_)
| ScriptSatisfactionError
| TapAnnexUnsupported
| UncompressedPubkey
Expand Down
8 changes: 5 additions & 3 deletions src/interpreter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use core::fmt;
use core::str::FromStr;

use bitcoin::hashes::{hash160, ripemd160, sha256, Hash};
use bitcoin::{absolute, secp256k1, sighash, taproot, Sequence, TxOut, Witness};
use bitcoin::{absolute, relative, secp256k1, sighash, taproot, Sequence, TxOut, Witness};

use crate::miniscript::context::{NoChecks, SigType};
use crate::miniscript::ScriptContext;
Expand Down Expand Up @@ -468,7 +468,7 @@ pub enum SatisfiedConstraint {
///Relative Timelock for CSV.
RelativeTimelock {
/// The value of RelativeTimelock
n: Sequence,
n: relative::LockTime,
},
///Absolute Timelock for CLTV.
AbsoluteTimelock {
Expand Down Expand Up @@ -1182,7 +1182,9 @@ mod tests {
let older_satisfied: Result<Vec<SatisfiedConstraint>, Error> = constraints.collect();
assert_eq!(
older_satisfied.unwrap(),
vec![SatisfiedConstraint::RelativeTimelock { n: Sequence::from_height(1000) }]
vec![SatisfiedConstraint::RelativeTimelock {
n: crate::RelLockTime::from_height(1000).into()
}]
);

//Check Sha256
Expand Down
28 changes: 14 additions & 14 deletions src/interpreter/stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

use bitcoin::blockdata::{opcodes, script};
use bitcoin::hashes::{hash160, ripemd160, sha256, Hash};
use bitcoin::{absolute, Sequence};
use bitcoin::{absolute, relative, Sequence};

use super::error::PkEvalErrInner;
use super::{verify_sersig, BitcoinKey, Error, HashLockType, KeySigPair, SatisfiedConstraint};
Expand Down Expand Up @@ -212,19 +212,14 @@ impl<'txin> Stack<'txin> {
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(),
)))
}
_ => return Some(Err(Error::AbsoluteLocktimeComparisonInvalid(*n, lock_time))),
};

if is_satisfied {
self.push(Element::Satisfied);
Some(Ok(SatisfiedConstraint::AbsoluteTimelock { n: *n }))
} else {
Some(Err(Error::AbsoluteLocktimeNotMet(n.to_consensus_u32())))
Some(Err(Error::AbsoluteLocktimeNotMet(*n)))
}
}

Expand All @@ -236,14 +231,19 @@ impl<'txin> Stack<'txin> {
/// booleans
pub(super) fn evaluate_older(
&mut self,
n: &Sequence,
age: Sequence,
n: &relative::LockTime,
sequence: Sequence,
) -> Option<Result<SatisfiedConstraint, Error>> {
if age >= *n {
self.push(Element::Satisfied);
Some(Ok(SatisfiedConstraint::RelativeTimelock { n: *n }))
if let Some(tx_locktime) = sequence.to_relative_lock_time() {
if n.is_implied_by(tx_locktime) {
self.push(Element::Satisfied);
Some(Ok(SatisfiedConstraint::RelativeTimelock { n: *n }))
} else {
Some(Err(Error::RelativeLocktimeNotMet(*n)))
}
} else {
Some(Err(Error::RelativeLocktimeNotMet(n.to_consensus_u32())))
// BIP 112: if the tx locktime has the disable flag set, fail CSV.
Some(Err(Error::RelativeLocktimeNotMet(*n)))
}
}

Expand Down

0 comments on commit 9b3d45d

Please sign in to comment.