fix: off-by-one in MedianTimePast::is_satisfied_by
What changed, and why it matters
This commit fixes a one-unit boundary error in how rust-bitcoin decides whether a time-based transaction lock has expired. The code previously treated 'lock time equal to median time past' as satisfied, but Bitcoin's BIP-113 rule requires strict less-than. A transaction built with rust-bitcoin using an equal-time lock could be accepted locally but then rejected by Bitcoin Core, causing inconsistent behavior, failed broadcasts, or mempool rejection.
Upgrade to the patched version. Review any code that constructs or validates absolute time locktimes to ensure it relies on the corrected strict-less-than semantics and does not assume equality means finality.
Security signals we found
Consensus-rule mismatch with Bitcoin Core (BIP-113)
Off-by-one boundary error in locktime validation
Potential transaction broadcast/mempool rejection
Fixes public issue #6373
Evidence from the diff
MedianTimePast::is_satisfied_by compared locktime to the median-time-past with <= instead of <. Per BIP-113 and Bitcoin Core’s IsFinalTx, a transaction with absolute time lock T is final only if T < MTP_of_previous_block. The patch changes the operator, updates the affected unit tests, and adds a BIP-113 compliance test covering T < MTP, T == MTP, and T > MTP.
Changed components
units/src/locktime/absolute/mod.rsMedianTimePast::is_satisfied_byabsolute time-based locktime validationInspect captured patch +41 / −5
diff --git a/units/src/locktime/absolute/mod.rs b/units/src/locktime/absolute/mod.rs
index 4f9da2bd..19ad9bce 100644
--- a/units/src/locktime/absolute/mod.rs
+++ b/units/src/locktime/absolute/mod.rs
@@ -719,8 +719,9 @@ impl MedianTimePast {
#[inline]
pub fn is_satisfied_by(self, time: Self) -> 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
+ // of the previous block - which is expected to be `time` here.
+ // This requires a strict less-than comparison (`<`) per BIP-113.
+ self < time
}
}
@@ -915,7 +916,8 @@ mod tests {
let height = Height::from_u32(800_000).unwrap();
assert!(!lock_by_time.is_satisfied_by(height, time_before));
- assert!(lock_by_time.is_satisfied_by(height, time));
+ // Strict less-than comparison (BIP-113) means equality is not satisfied.
+ assert!(!lock_by_time.is_satisfied_by(height, time));
assert!(lock_by_time.is_satisfied_by(height, time_after));
}
@@ -1076,14 +1078,48 @@ mod tests {
fn median_time_past_is_satisfied_by() {
let mtp = MedianTimePast::from_u32(500_000_001).unwrap();
- // lock is satisfied if transaction can go in the next block (locktime <= mtp).
+ // lock is satisfied if transaction can go in the next block (locktime < mtp).
let locktime = MedianTimePast::from_u32(500_000_000).unwrap();
assert!(locktime.is_satisfied_by(mtp));
+
+ // It is not satisfied if the lock time is equal to the median time past (BIP-113).
let locktime = MedianTimePast::from_u32(500_000_001).unwrap();
- assert!(locktime.is_satisfied_by(mtp));
+ assert!(!locktime.is_satisfied_by(mtp));
// It is not satisfied if the lock time is after the median time past.
let locktime = MedianTimePast::from_u32(500_000_002).unwrap();
assert!(!locktime.is_satisfied_by(mtp));
}
+
+ #[test]
+ fn median_time_past_satisfaction_bip113_compliance() {
+ let mtp_reference = MedianTimePast::from_u32(500_000_001).unwrap();
+
+ // locktime is strictly less than MTP (T < MTP)
+ let early_locktime = MedianTimePast::from_u32(500_000_000).unwrap();
+ assert!(
+ early_locktime.is_satisfied_by(mtp_reference),
+ "Locktime strictly less than MTP ({} < {}) must be satisfied",
+ early_locktime,
+ mtp_reference
+ );
+
+ // locktime is exactly equal to MTP (T == MTP)
+ let equal_locktime = MedianTimePast::from_u32(500_000_001).unwrap();
+ assert!(
+ !equal_locktime.is_satisfied_by(mtp_reference),
+ "Locktime equal to MTP ({} == {}) must not be satisfied (BIP-113 off-by-one violation)",
+ equal_locktime,
+ mtp_reference
+ );
+
+ // locktime is strictly greater than MTP (T > MTP)
+ let future_locktime = MedianTimePast::from_u32(500_000_002).unwrap();
+ assert!(
+ !future_locktime.is_satisfied_by(mtp_reference),
+ "Locktime greater than MTP ({} > {}) must not be satisfied",
+ future_locktime,
+ mtp_reference
+ );
+ }
}
Why this scored 66/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.