Skip to content

Commit

Permalink
Merge #414: Improve timelock code
Browse files Browse the repository at this point in the history
b084010 Use terse functional programming terms (Tobin C. Harding)
6f3303d Improve docs on TimelockInfo (Tobin C. Harding)
b2f6ef0 Add unit test for combine_threshold (Tobin C. Harding)
a36f608 Be uniform in spelling of timelock (Tobin C. Harding)
51de643 Rename comibine methods (Tobin C. Harding)
ef6803f Use > 1 instead of >= 2 (Tobin C. Harding)
d1fdbaa Use LOCKTIME_THRESHOLD same as bitcoin core (Tobin C. Harding)

Pull request description:

  Done while working on a [timelock module](rust-bitcoin/rust-bitcoin#994). This is all the initial patches (except one) from #408 (which is a PR displaying usage of the new timelock module).

  Note, does _not_ do the 'make `TimelockInfo` fields pub crate' change - I was unsure if this was correct.

Top commit has no ACKs.

Tree-SHA512: aa54e2d884f7cb1fb5dcb2d828ada29830ac4a7a09b04797e6e2fb448df476cbb31345841735e6cf4d5b7b1f6783781859778805fffef708f259fe780c6ba677
  • Loading branch information
sanket1729 committed May 19, 2022
2 parents 5649628 + b084010 commit d40003c
Show file tree
Hide file tree
Showing 8 changed files with 111 additions and 88 deletions.
8 changes: 4 additions & 4 deletions src/interpreter/mod.rs
Expand Up @@ -485,12 +485,12 @@ pub enum SatisfiedConstraint {
preimage: [u8; 32],
},
///Relative Timelock for CSV.
RelativeTimeLock {
RelativeTimelock {
/// The value of RelativeTimelock
time: u32,
},
///Absolute Timelock for CLTV.
AbsoluteTimeLock {
AbsoluteTimelock {
/// The value of Absolute timelock
time: u32,
},
Expand Down Expand Up @@ -1197,7 +1197,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 { time: 1000 }]
);

//Check Older
Expand All @@ -1207,7 +1207,7 @@ mod tests {
let older_satisfied: Result<Vec<SatisfiedConstraint>, Error> = constraints.collect();
assert_eq!(
older_satisfied.unwrap(),
vec![SatisfiedConstraint::RelativeTimeLock { time: 1000 }]
vec![SatisfiedConstraint::RelativeTimelock { time: 1000 }]
);

//Check Sha256
Expand Down
4 changes: 2 additions & 2 deletions src/interpreter/stack.rs
Expand Up @@ -235,7 +235,7 @@ impl<'txin> Stack<'txin> {
) -> Option<Result<SatisfiedConstraint, Error>> {
if age >= *n {
self.push(Element::Satisfied);
Some(Ok(SatisfiedConstraint::AbsoluteTimeLock { time: *n }))
Some(Ok(SatisfiedConstraint::AbsoluteTimelock { time: *n }))
} else {
Some(Err(Error::AbsoluteLocktimeNotMet(*n)))
}
Expand All @@ -254,7 +254,7 @@ impl<'txin> Stack<'txin> {
) -> Option<Result<SatisfiedConstraint, Error>> {
if height >= *n {
self.push(Element::Satisfied);
Some(Ok(SatisfiedConstraint::RelativeTimeLock { time: *n }))
Some(Ok(SatisfiedConstraint::RelativeTimelock { time: *n }))
} else {
Some(Err(Error::RelativeLocktimeNotMet(*n)))
}
Expand Down
6 changes: 3 additions & 3 deletions src/miniscript/analyzable.rs
Expand Up @@ -41,7 +41,7 @@ pub enum AnalysisError {
/// Miniscript contains at least one path that exceeds resource limits
BranchExceedResouceLimits,
/// Contains a combination of heightlock and timelock
HeightTimeLockCombination,
HeightTimelockCombination,
/// Malleable script
Malleable,
}
Expand All @@ -58,7 +58,7 @@ impl fmt::Display for AnalysisError {
AnalysisError::BranchExceedResouceLimits => {
f.write_str("At least one spend path exceeds the resource limits(stack depth/satisfaction size..)")
}
AnalysisError::HeightTimeLockCombination => {
AnalysisError::HeightTimelockCombination => {
f.write_str("Contains a combination of heightlock and timelock")
}
AnalysisError::Malleable => f.write_str("Miniscript is malleable")
Expand Down Expand Up @@ -128,7 +128,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
} else if self.has_repeated_keys() {
Err(AnalysisError::RepeatedPubkeys)
} else if self.has_mixed_timelocks() {
Err(AnalysisError::HeightTimeLockCombination)
Err(AnalysisError::HeightTimelockCombination)
} else {
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion src/miniscript/limits.rs
Expand Up @@ -17,7 +17,7 @@ pub const MAX_STANDARD_P2WSH_SCRIPT_SIZE: usize = 3600;
/// The Threshold for deciding whether `nLockTime` is interpreted as
/// time or height.
// https://github.com/bitcoin/bitcoin/blob/9ccaee1d5e2e4b79b0a7c29aadb41b97e4741332/src/script/script.h#L39
pub const HEIGHT_TIME_THRESHOLD: u32 = 500_000_000;
pub const LOCKTIME_THRESHOLD: u32 = 500_000_000;

/// Bit flag for deciding whether sequence number is
/// interpreted as height or time
Expand Down
4 changes: 2 additions & 2 deletions src/miniscript/satisfy.rs
Expand Up @@ -28,7 +28,7 @@ use bitcoin::secp256k1::XOnlyPublicKey;
use bitcoin::util::taproot::{ControlBlock, LeafVersion, TapLeafHash};

use crate::miniscript::limits::{
HEIGHT_TIME_THRESHOLD, SEQUENCE_LOCKTIME_DISABLE_FLAG, SEQUENCE_LOCKTIME_TYPE_FLAG,
LOCKTIME_THRESHOLD, SEQUENCE_LOCKTIME_DISABLE_FLAG, SEQUENCE_LOCKTIME_TYPE_FLAG,
};
use crate::util::witness_size;
use crate::{Miniscript, MiniscriptKey, ScriptContext, Terminal, ToPublicKey};
Expand Down Expand Up @@ -155,7 +155,7 @@ pub struct After(pub u32);
impl<Pk: MiniscriptKey + ToPublicKey> Satisfier<Pk> for After {
fn check_after(&self, n: u32) -> bool {
// if n > self.0; we will be returning false anyways
if n < HEIGHT_TIME_THRESHOLD && self.0 >= HEIGHT_TIME_THRESHOLD {
if n < LOCKTIME_THRESHOLD && self.0 >= LOCKTIME_THRESHOLD {
false
} else {
n <= self.0
Expand Down

0 comments on commit d40003c

Please sign in to comment.