pub(crate) struct CanonicalEntry<P> {
pub(crate) tx: Arc<Transaction>,
pub(crate) pos: P,
- pub(crate) mtp: Option<u32>,
+ pub(crate) prev_mtp: Option<u32>,
}
/// A single canonical transaction with its position.
pub txid: Txid,
/// The full transaction.
pub tx: Arc<Transaction>,
- /// The median-time-past at the confirmation height, if computed.
- ///
- /// This is `Some` only when the transaction is confirmed and MTP computation was
- /// enabled via [`CanonicalViewTask::with_mtp`](crate::CanonicalViewTask::with_mtp).
- pub mtp: Option<u32>,
+ /// The median-time-past of the block *preceding* the confirmation height, if computed.
+ ///
+ /// This is the MTP at `confirmation_height - 1` (saturating at genesis), which is the
+ /// reference value used to evaluate BIP-68 relative timelocks on spends of this
+ /// transaction's outputs — matching Bitcoin Core's `CalculateSequenceLocks`. It is *not*
+ /// the MTP at the confirmation height itself.
+ ///
+ /// `None` means the value is unknown, which happens whenever any of these hold:
+ ///
+ /// - MTP computation was not enabled — see
+ /// [`CanonicalViewTask::with_mtp`](crate::CanonicalViewTask::with_mtp).
+ /// - The transaction is not confirmed.
+ /// - The transaction is confirmed only *transitively* — through a descendant's anchor, so
+ /// [`pos`](Self::pos) is [`Confirmed`](crate::ChainPosition::Confirmed) with
+ /// `transitively: Some(_)`. Its own confirmation height genuinely isn't known.
+ /// - The chain could not resolve the full 11-block window the MTP medians over.
+ ///
+ /// So a confirmed transaction can have `None` here even with MTP enabled; do not read
+ /// `None` as "MTP was off", and do not treat it as an absent or satisfied timelock.
+ pub prev_mtp: Option<u32>,
}
impl<P: Ord> Ord for CanonicalTx<P> {
pub spent_by: Option<(P, Txid)>,
/// Whether this output is on a coinbase transaction.
pub is_on_coinbase: bool,
- /// The median-time-past at the confirmation height, if computed.
+ /// The median-time-past of the block *preceding* the confirmation height, if computed.
+ ///
+ /// This is the MTP at `confirmation_height - 1` (saturating at genesis), which is the
+ /// reference value used to evaluate BIP-68 relative timelocks on spends of this output —
+ /// matching Bitcoin Core's `CalculateSequenceLocks`. It is *not* the MTP at the
+ /// confirmation height itself.
///
- /// This is `Some` only when the output's transaction is confirmed and MTP computation
- /// was enabled via [`CanonicalViewTask::with_mtp`](crate::CanonicalViewTask::with_mtp).
- pub mtp: Option<u32>,
+ /// `None` means unknown, not "no timelock" — see
+ /// [`CanonicalTx::prev_mtp`](crate::CanonicalTx::prev_mtp) for the cases that produce it,
+ /// which include an output whose transaction is confirmed only transitively.
+ pub prev_mtp: Option<u32>,
}
impl<P: Ord> Ord for CanonicalTxOut<P> {
pos: entry.pos.clone(),
txid,
tx: entry.tx.clone(),
- mtp: entry.mtp,
+ prev_mtp: entry.prev_mtp,
})
}
txout: txout.clone(),
spent_by,
is_on_coinbase: entry.tx.is_coinbase(),
- mtp: entry.mtp,
+ prev_mtp: entry.prev_mtp,
})
}
pos: entry.pos.clone(),
txid,
tx: entry.tx.clone(),
- mtp: entry.mtp,
+ prev_mtp: entry.prev_mtp,
}
})
}
canonical_entry.insert(CanonicalEntry {
tx: tx.clone(),
pos: this_reason,
- mtp: None,
+ prev_mtp: None,
});
Some(this_txid)
},
ChainPosition::Confirmed { .. }
));
- // Per-tx MTP at height 5: median of timestamps at heights 0..=5
- // Timestamps: 100, 200, 300, 400, 500, 600 → sorted, median = ts[len/2] = ts[3] = 400
+ // Per-tx `prev_mtp` for a tx confirmed at height 5: the MTP of the block *preceding*
+ // the confirmation height (BIP-68), i.e. MTP at height 4 = median of timestamps at
+ // heights 0..=4.
+ // Timestamps: 100, 200, 300, 400, 500 → sorted, median = ts[len/2] = ts[2] = 300.
+ // (Was 400 when the buggy code medianed heights 0..=5 at the confirmation height itself.)
let canon_tx = view.txs().next().unwrap();
- assert_eq!(canon_tx.mtp, Some(400));
+ assert_eq!(canon_tx.prev_mtp, Some(300));
// MTP at tip height (11): median of timestamps at heights 1..=11
// Timestamps: 200, 300, 400, 500, 600, 700, 800, 900, 1000, 1100, 1200
impl<'g, A: Anchor, B: ToBlockHash + ToBlockTime> CanonicalViewTask<'g, A, B> {
/// Enable MTP (median-time-past) computation.
///
- /// When enabled, the task will fetch additional blocks needed to compute MTP values
- /// for each confirmed height and the tip height. The resulting [`CanonicalView`] will
- /// have per-tx MTP on [`CanonicalTx::mtp`](crate::CanonicalTx::mtp) and the tip MTP
- /// accessible via [`tip_mtp()`](crate::Canonical::tip_mtp).
+ /// When enabled, the task will fetch additional blocks needed to compute MTP values.
+ /// For each confirmed tx it computes the MTP of the block *preceding* the confirmation
+ /// height (per BIP-68, matching Bitcoin Core's `CalculateSequenceLocks`), exposed on
+ /// [`CanonicalTx::prev_mtp`](crate::CanonicalTx::prev_mtp). For the tip it computes
+ /// MTP(tip.height) (per BIP-113), accessible via
+ /// [`tip_mtp()`](crate::Canonical::tip_mtp).
pub fn with_mtp(mut self) -> Self {
self.extract_time = Some(B::to_blocktime);
self
}
// Collect all MTP heights needed (an 11-block window per target height).
+ //
+ // For the tip we need MTP(tip.height) (BIP-113). For each confirmed tx we need
+ // the MTP of the block *preceding* the confirmation height (BIP-68), so we push
+ // `confirmation_height - 1` (saturating at genesis) — the same value read in
+ // `finish`.
let mut mtp_heights = Vec::<u32>::new();
mtp_heights.push(self.tip.height);
for anchor in self.direct_anchors.values() {
- mtp_heights.push(anchor.confirmation_height_upper_bound());
+ mtp_heights.push(anchor.confirmation_height_upper_bound().saturating_sub(1));
}
let required: Vec<u32> = mtp_heights
.into_iter()
anchor: anchor.clone(),
transitively: None,
};
- let mtp = extract_time
- .and_then(|f| compute_mtp_at(anchor.confirmation_height_upper_bound(), f));
- view_txs.insert(*txid, CanonicalEntry { tx, pos, mtp });
+ // BIP-68 evaluates relative timelocks against the MTP of the block *before* the
+ // coin's height (Core's `std::max(nCoinHeight - 1, 0)`), so compute MTP at
+ // `confirmation_height - 1`, saturating to genesis for coins at height 0.
+ let prev_mtp = extract_time.and_then(|f| {
+ compute_mtp_at(
+ anchor.confirmation_height_upper_bound().saturating_sub(1),
+ f,
+ )
+ });
+ view_txs.insert(*txid, CanonicalEntry { tx, pos, prev_mtp });
continue;
}
CanonicalEntry {
tx: tx.clone(),
pos,
- // No MTP since we do not know confirmation height.
- mtp: None,
+ // No MTP: this stage handles txs without a direct anchor, so even when the
+ // position is `Confirmed` (transitively, via a descendant's anchor) this
+ // tx's own confirmation height is unknown and there is no block to median
+ // around.
+ prev_mtp: None,
},
);
}
///
/// Requires `D: ToBlockTime` (e.g. `LocalChain<Header>`).
///
- /// The resulting [`CanonicalView`] will have per-tx MTP on
- /// [`CanonicalTx::mtp`](crate::CanonicalTx::mtp) and the tip MTP accessible via
+ /// For each confirmed transaction this computes the MTP of the block *preceding* the
+ /// confirmation height (the BIP-68 reference for relative timelocks), exposed on
+ /// [`CanonicalTx::prev_mtp`](crate::CanonicalTx::prev_mtp). For the tip it computes
+ /// MTP(tip.height) (the BIP-113 reference for absolute timelocks), accessible via
/// [`tip_mtp()`](crate::Canonical::tip_mtp).
+ ///
+ /// See [`CanonicalViewTask::with_mtp`](crate::CanonicalViewTask::with_mtp) for the block
+ /// data this requires.
pub fn canonicalize_with_mtp<A: Anchor>(
&self,
tx_graph: &TxGraph<A>,
LocalChain::from_blocks(blocks).expect("chain connects from genesis")
}
+/// Insert a transaction into `tx_graph` spending a distinct parent (identified by `seed`) and
+/// anchor it at `height`.
+///
+/// Returns the txid of the inserted transaction.
+fn confirm_tx_at(
+ tx_graph: &mut TxGraph<ConfirmationBlockTime>,
+ chain: &LocalChain<Header>,
+ seed: u8,
+ height: u32,
+) -> bitcoin::Txid {
+ let tx = Transaction {
+ input: vec![TxIn {
+ previous_output: OutPoint::new(bitcoin::Txid::from_byte_array([seed; 32]), 0),
+ ..Default::default()
+ }],
+ output: vec![TxOut {
+ value: Amount::from_sat(50_000),
+ script_pubkey: ScriptBuf::new(),
+ }],
+ ..new_tx(1)
+ };
+ let txid = tx.compute_txid();
+ let _ = tx_graph.insert_tx(tx);
+ let _ = tx_graph.insert_anchor(
+ txid,
+ ConfirmationBlockTime {
+ block_id: chain.get(height).unwrap().block_id(),
+ confirmation_time: 0,
+ },
+ );
+ txid
+}
+
#[test]
fn canonicalize_with_mtp_computes_median_time_past() {
// A 13-block chain (heights 0..=12) where each block's time equals its height.
// Tip is at height 12, so its MTP is the median of heights 2..=12, which is 7.
assert_eq!(view.tip_mtp(), Some(7));
- // The transaction is confirmed at height 11, so its MTP is the median of heights 1..=11 == 6.
+ // The transaction is confirmed at height 11. Its `prev_mtp` is the MTP of the block
+ // *preceding* the confirmation height (BIP-68), i.e. MTP at height 10 = median of heights
+ // 0..=10 == 5. (Was 6 when the buggy code medianed heights 1..=11 at height 11 itself.)
let canonical_tx = view
.txs()
.find(|c| c.txid == txid)
.expect("tx is canonical");
assert!(matches!(canonical_tx.pos, ChainPosition::Confirmed { .. }));
- assert_eq!(canonical_tx.mtp, Some(6));
+ assert_eq!(canonical_tx.prev_mtp, Some(5));
+}
+
+#[test]
+fn prev_mtp_saturates_for_low_confirmation_heights() {
+ // A 13-block chain (heights 0..=12) where each block's time equals its height.
+ let chain = chain_with_time_equal_to_height(12);
+ let mut tx_graph = TxGraph::default();
+
+ // Confirmed at genesis (height 0): `confirmation_height - 1` saturates to 0, matching
+ // Core's `std::max(nCoinHeight - 1, 0)`. prev_mtp = MTP(0) = median of heights 0..=0 == 0.
+ let genesis_txid = confirm_tx_at(&mut tx_graph, &chain, 1, 0);
+
+ // Confirmed at height 3: prev_mtp = MTP(2) = median of heights 0..=2 == 1. The MTP window
+ // start (`2 - 10`) also saturates to 0. (The buggy code would have medianed heights 0..=3
+ // at height 3 == 2.)
+ let low_txid = confirm_tx_at(&mut tx_graph, &chain, 2, 3);
+
+ let view = chain.canonicalize_with_mtp(&tx_graph, chain.tip().block_id(), Default::default());
+
+ let genesis_tx = view
+ .txs()
+ .find(|c| c.txid == genesis_txid)
+ .expect("tx is canonical");
+ assert_eq!(genesis_tx.prev_mtp, Some(0));
+
+ let low_tx = view
+ .txs()
+ .find(|c| c.txid == low_txid)
+ .expect("tx is canonical");
+ assert_eq!(low_tx.prev_mtp, Some(1));
+}
+
+#[test]
+fn prev_mtp_differs_from_tip_mtp() {
+ // A 13-block chain (heights 0..=12) where each block's time equals its height.
+ let chain = chain_with_time_equal_to_height(12);
+ let mut tx_graph = TxGraph::default();
+
+ // Confirmed at height 11, with the tip at height 12: prev_mtp = MTP(10) = 5, while
+ // tip_mtp = MTP(12) = 7. These are distinguishable, so `prev_mtp` (BIP-68, the block before
+ // the coin) is clearly not the same reference as `tip_mtp` (BIP-113, the tip).
+ let txid = confirm_tx_at(&mut tx_graph, &chain, 1, 11);
+
+ let view = chain.canonicalize_with_mtp(&tx_graph, chain.tip().block_id(), Default::default());
+
+ let canonical_tx = view.txs().find(|c| c.txid == txid).expect("tx is canonical");
+ assert_eq!(canonical_tx.prev_mtp, Some(5));
+ assert_eq!(view.tip_mtp(), Some(7));
+ assert_ne!(canonical_tx.prev_mtp, view.tip_mtp());
}
projects / bdk / commitdiff