Fix off-by-one-bug in absolute locktime

When checking a locktime against block height we add 1 because when the
next block is being validated that is what the height will be and
`is_satisfied_by` is defined to return true if a transaction with this
locktime can be included in the next block.

As we have in `relative`, and for the same reasons, add an API to the
absolute `Height` and `MedianTimePast`: `is_satisfied_by`. Also add
`is_satisfied_by_{height, time}` variants to `absolute::LockTime`.
Call through to the new functions in `units`.
This commit is contained in:
Tobin C. Harding 2025-05-11 10:56:13 +10:00
parent 3ffdc54ca5
commit 727047bd39
No known key found for this signature in database
GPG Key ID: 40BF9E4C269D6607
2 changed files with 128 additions and 4 deletions

View File

@ -233,13 +233,21 @@ impl LockTime {
/// is satisfied if a transaction with `nLockTime` ([`Transaction::lock_time`]) set to
/// `height`/`time` is valid.
///
/// If `height` and `mtp` represent the current chain tip then a transaction with this
/// locktime can be broadcast for inclusion in the next block.
///
/// If you do not have, or do not wish to calculate, both parameters consider using:
///
/// * [`is_satisfied_by_height()`](absolute::LockTime::is_satisfied_by_height)
/// * [`is_satisfied_by_time()`](absolute::LockTime::is_satisfied_by_time)
///
/// # Examples
///
/// ```no_run
/// # use bitcoin_primitives::absolute;
/// // Can be implemented if block chain data is available.
/// fn get_height() -> absolute::Height { todo!("return the current block height") }
/// fn get_time() -> absolute::MedianTimePast { todo!("return the current block time") }
/// fn get_time() -> absolute::MedianTimePast { todo!("return the current block MTP") }
///
/// let n = absolute::LockTime::from_consensus(741521); // `n OP_CHEKCLOCKTIMEVERIFY`.
/// if n.is_satisfied_by(get_height(), get_time()) {
@ -247,12 +255,43 @@ impl LockTime {
/// }
/// ````
#[inline]
pub fn is_satisfied_by(self, height: Height, time: MedianTimePast) -> bool {
pub fn is_satisfied_by(self, height: Height, mtp: MedianTimePast) -> bool {
match self {
LockTime::Blocks(blocks) => blocks.is_satisfied_by(height),
LockTime::Seconds(time) => time.is_satisfied_by(mtp),
}
}
/// Returns true if a transaction with this locktime can be spent in the next block.
///
/// If `height` is the current block height of the chain then a transaction with this locktime
/// can be broadcast for inclusion in the next block.
///
/// # Errors
///
/// Returns an error if this lock is not lock-by-height.
#[inline]
pub fn is_satisfied_by_height(self, height: Height) -> Result<bool, IncompatibleHeightError> {
use LockTime as L;
match self {
L::Blocks(n) => n <= height,
L::Seconds(n) => n <= time,
L::Blocks(blocks) => Ok(blocks.is_satisfied_by(height)),
L::Seconds(time) => Err(IncompatibleHeightError { lock: time, incompatible: height }),
}
}
/// Returns true if a transaction with this locktime can be included in the next block.
///
/// # Errors
///
/// Returns an error if this lock is not lock-by-time.
#[inline]
pub fn is_satisfied_by_time(self, mtp: MedianTimePast) -> Result<bool, IncompatibleTimeError> {
use LockTime as L;
match self {
L::Seconds(time) => Ok(time.is_satisfied_by(mtp)),
L::Blocks(blocks) => Err(IncompatibleTimeError { lock: blocks, incompatible: mtp }),
}
}
@ -407,6 +446,68 @@ impl<'de> serde::Deserialize<'de> for LockTime {
}
}
/// Tried to satisfy a lock-by-time lock using a height value.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct IncompatibleHeightError {
/// The inner value of the lock-by-time lock.
lock: MedianTimePast,
/// Attempted to satisfy a lock-by-time lock with this height.
incompatible: Height,
}
impl IncompatibleHeightError {
/// Returns the value of the lock-by-time lock.
pub fn lock(&self) -> MedianTimePast { self.lock }
/// Returns the height that was erroneously used to try and satisfy a lock-by-time lock.
pub fn incompatible(&self) -> Height { self.incompatible }
}
impl fmt::Display for IncompatibleHeightError {
#[inline]
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(
f,
"tried to satisfy a lock-by-time lock {} with height: {}",
self.lock, self.incompatible
)
}
}
#[cfg(feature = "std")]
impl std::error::Error for IncompatibleHeightError {}
/// Tried to satisfy a lock-by-height lock using a height value.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct IncompatibleTimeError {
/// The inner value of the lock-by-height lock.
lock: Height,
/// Attempted to satisfy a lock-by-height lock with this MTP.
incompatible: MedianTimePast,
}
impl IncompatibleTimeError {
/// Returns the value of the lock-by-height lock.
pub fn lock(&self) -> Height { self.lock }
/// Returns the MTP that was erroneously used to try and satisfy a lock-by-height lock.
pub fn incompatible(&self) -> MedianTimePast { self.incompatible }
}
impl fmt::Display for IncompatibleTimeError {
#[inline]
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(
f,
"tried to satisfy a lock-by-height lock {} with MTP: {}",
self.lock, self.incompatible
)
}
}
#[cfg(feature = "std")]
impl std::error::Error for IncompatibleTimeError {}
#[cfg(feature = "arbitrary")]
impl<'a> Arbitrary<'a> for LockTime {
fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result<Self> {

View File

@ -83,6 +83,17 @@ impl Height {
/// Converts this [`Height`] to a raw `u32` value.
#[inline]
pub const fn to_u32(self) -> u32 { self.0 }
/// Returns true if a transaction with this locktime can be included in the next block.
///
/// `self` is value of the `LockTime` and if `height` is the current chain tip then
/// a transaction with this lock can be broadcast for inclusion in the next block.
#[inline]
pub fn is_satisfied_by(self, height: Height) -> bool {
// Use u64 so that there can be no overflow.
let next_block_height = u64::from(height.to_u32()) + 1;
u64::from(self.to_u32()) <= next_block_height
}
}
impl fmt::Display for Height {
@ -217,6 +228,18 @@ impl MedianTimePast {
/// Converts this [`MedianTimePast`] to a raw `u32` value.
#[inline]
pub const fn to_u32(self) -> u32 { self.0 }
/// Returns true if a transaction with this locktime can be included in the next block.
///
/// `self`is the value of the `LockTime` and if `time` is the median time past of the block at
/// the chain tip then a transaction with this lock can be broadcast for inclusion in the next
/// block.
#[inline]
pub fn is_satisfied_by(self, time: MedianTimePast) -> bool {
// The locktime check in Core during block validation uses the MTP
// of the previous block - which is the expected to be `time` here.
self <= time
}
}
impl fmt::Display for MedianTimePast {