Skip to content

Commit 287f17c

Browse files
evanlinjinclaude
andcommitted
fix!: Correct absolute timelock boundary checks
Fix off-by-one error in absolute timelock checks. Bitcoin Core's `IsFinalTx` uses strict less-than: tx is final when `nLockTime < MTP` (for time) or `nLockTime < blockHeight` (for height). The previous checks used `>` but should use `>=` to match Core's behavior. Also add 4 new time-based timelock tests that verify BDK's predictions match Bitcoin Core's actual broadcast acceptance at exact boundaries: - test_absolute_time_timelock_logic (integration) - test_relative_time_timelock_logic (integration) - test_is_time_timelocked_absolute_unit - test_is_time_timelocked_relative_unit Patch miniscript to fix Plan::satisfy() not appending witnessScript for P2WSH descriptors (rust-bitcoin/rust-miniscript#XXX). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent c20ac77 commit 287f17c

3 files changed

Lines changed: 703 additions & 10 deletions

File tree

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ bdk_testenv = { git = "https://github.com/evanlinjin/bdk", branch = "feature/bit
4747
bdk_chain = { git = "https://github.com/evanlinjin/bdk", branch = "feature/bitcoind_rpc_emit_header" }
4848
bdk_core = { git = "https://github.com/evanlinjin/bdk", branch = "feature/bitcoind_rpc_emit_header" }
4949
bdk_bitcoind_rpc = { git = "https://github.com/evanlinjin/bdk", branch = "feature/bitcoind_rpc_emit_header" }
50+
miniscript = { git = "https://github.com/evanlinjin/rust-miniscript", branch = "fix/plan-satisfy-does-not-append-witness-script-for-p2wsh" }
5051

5152
# bdk_chain = { git = "https://github.com/bitcoindevkit/bdk", branch = "master" }
5253
# bdk_core = { git = "https://github.com/bitcoindevkit/bdk", branch = "master" }

src/input.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -368,12 +368,17 @@ impl Input {
368368
.checked_add(1)
369369
.expect("must not overflow");
370370
if let Some(absolute::LockTime::Blocks(lt_height)) = self.plan.absolute_timelock() {
371-
return lt_height.to_consensus_u32() > spending_height;
371+
// Bitcoin Core's `IsFinalTx` uses strict less-than: a tx is final (unlocked) when
372+
// `nLockTime < blockHeight`. This means `nLockTime = 100` is first spendable in
373+
// block 101, not block 100. We return "locked" when the inverse is true.
374+
return lt_height.to_consensus_u32() >= spending_height;
372375
}
373376

374377
if let (Some(relative::LockTime::Blocks(lt_height)), Some(conf_status)) =
375378
(self.plan.relative_timelock(), self.status)
376379
{
380+
// BIP 68: relative lock is satisfied when `height_diff >= lock_value`.
381+
// We return "locked" when `lock_value > height_diff`.
377382
let height_diff = spending_height.saturating_sub(conf_status.height.to_consensus_u32());
378383
return lt_height.to_consensus_u32() > height_diff;
379384
}
@@ -388,12 +393,17 @@ impl Input {
388393
/// `tip_mtp` is `MTP(tip)`, or `MTP(spending_block - 1)`, as per BIP-0068.
389394
pub fn is_time_timelocked(&self, tip_mtp: absolute::Time) -> Option<bool> {
390395
if let Some(absolute::LockTime::Seconds(lt_time)) = self.plan.absolute_timelock() {
391-
return Some(lt_time.to_consensus_u32() > tip_mtp.to_consensus_u32());
396+
// Bitcoin Core's `IsFinalTx` (with BIP 113) uses strict less-than: a tx is final
397+
// (unlocked) when `nLockTime < MTP`. This means `nLockTime = T` is first spendable
398+
// when `MTP > T`, not when `MTP == T`. We return "locked" when the inverse is true.
399+
return Some(lt_time.to_consensus_u32() >= tip_mtp.to_consensus_u32());
392400
}
393401

394402
if let (Some(relative::LockTime::Time(lt_time)), Some(conf_status)) =
395403
(self.plan.relative_timelock(), self.status)
396404
{
405+
// BIP 68: relative time lock is satisfied when `time_diff >= lock_value * 512`.
406+
// We return "locked" when `lock_value * 512 > time_diff`.
397407
let time_diff = tip_mtp
398408
.to_consensus_u32()
399409
.saturating_sub(conf_status.prev_mtp?.to_consensus_u32());

0 commit comments

Comments
 (0)