What the developer wrote
Authored by Tobin C. Harding
units: Remove incorrect code comment
In #6384 we change `<=` to `<`. While reviewing I thought this change
was incorrect at the time. After deeper consideration I see my confusion.
However the code comment added in the linked PR is definitely
incorrect and contradicts the rustdoc comments on the function.
`time` is the MTP of the chain tip. This is what the rustdocs say and
it is also what is stated in `validation.cpp` in Core:
// BIP113 requires that time-locked transactions have nLockTime set to
// less than the median time of the previous block they're contained in.
// When the next block is created its previous block will be the current
// chain tip, so we use that to calculate the median time passed to
// IsFinalTx().
const int64_t nBlockTime{active_chain_tip.GetMedianTimePast()};
return IsFinalTx(tx, nBlockHeight, nBlockTime);
`IsFinalTx` contains the equivalent lock time check that
`is_satisfied_by` calculates. This is exactly why we changed to use
`<` to match that function:
```
bool IsFinalTx(const CTransaction &tx, int nBlockHeight, int64_t nBlockTime)
{
if (tx.nLockTime == 0)
return true;
if ((int64_t)tx.nLockTime < ((int64_t)tx.nLockTime < LOCKTIME_THRESHOLD ? (int64_t)nBlockHeight : nBlockTime))
return true;
```
Remove the code comment.
What changed, and why it matters
This commit only deletes an incorrect explanatory comment. The actual code behavior does not change at all. It is a documentation cleanup, not a security fix.
No action needed; this is a non-functional documentation-only change.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch removes a six-line inline comment in units/src/locktime/absolute/mod.rs that incorrectly described the median-time-past comparison in MedianTimePast::is_satisfied_by. The function body remains self < time, identical to the pre-patch version. The commit message explains that the comment contradicted the rustdoc and Bitcoin Core’s IsFinalTx logic, and that the < operator is correct per BIP-113. No code logic is modified.
Changed components
units/src/locktime/absolute/mod.rsInspect captured patch +1 / −6
diff --git a/units/src/locktime/absolute/mod.rs b/units/src/locktime/absolute/mod.rs
index f152005a..93575516 100644
--- a/units/src/locktime/absolute/mod.rs
+++ b/units/src/locktime/absolute/mod.rs
@@ -694,12 +694,7 @@ impl MedianTimePast {
/// 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: Self) -> bool {
- // The locktime check in Core during block validation uses the MTP
- // of the previous block - which is expected to be `time` here.
- // This requires a strict less-than comparison (`<`) per BIP-113.
- self < time
- }
+ pub fn is_satisfied_by(self, time: Self) -> bool { self < time }
}
crate::internal_macros::impl_fmt_traits_for_u32_wrapper!(MedianTimePast);
Why this scored 15/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.