From 760d3689a2641957089af13be90619cb4f9df499 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Wed, 6 May 2026 12:49:46 +0000 Subject: [PATCH 1/9] refactor(core,chain)!: redesign `ChainQuery` to sans-io `ChainTask` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Exposing median-time-past (MTP) on canonical views needs block timestamps, but the `ChainQuery` interface can't carry them: its response is `ChainResponse = Option` — only a hash and height. Change the interface so the driver returns block *data* by height, generic over a block type `B` (default `BlockHash`, usable as a full `Header`). The driving call also moves from `next_query() -> Option<…>` to `poll() -> TaskProgress`, whose enum expresses states `next_query` couldn't: incremental progress (`Advanced`) and a `Blocked` state that leaves room for a future streaming driver. This keeps the multi-stage canonicalization a clean poll loop rather than an internal one. With block data flowing back, `CanonicalView` gains MTP: per-output on `CanonicalTxOut::prev_mtp`, and for the tip via `Canonical::tip_mtp`, both computed only when opted into with `CanonicalViewTask::with_mtp()`. `prev_mtp` is the MTP of the block *preceding* the output's confirmation height, not the MTP at that height. That is the reference BIP-68 evaluates relative timelocks against, matching Bitcoin Core's `CalculateSequenceLocks`: GetAncestor(std::max(nCoinHeight - 1, 0))->GetMedianTimePast() so it is computed at `confirmation_height.saturating_sub(1)`, the `saturating_sub` matching Core's `max(…, 0)`. `tip_mtp` is MTP at the tip height, which is the BIP-113 reference for absolute timelocks. MTP is deliberately on `CanonicalTxOut` only, not `CanonicalTx`: a relative timelock is evaluated against the coin being spent, and a transaction has no coin. `None` means the value is unknown — MTP not enabled, the output unconfirmed, its transaction confirmed only transitively, or an 11-block window the chain could not fully resolve — and must never be read as an absent or satisfied timelock. Mechanically: rename `ChainQuery` to `ChainTask`; remove `ChainRequest`/`ChainResponse`; add a `BlockQueries` helper for request/resolve tracking; add `LocalChain::run_task` and the `canonicalize{,_with_mtp}()` convenience methods. Tests run `canonicalize_with_mtp` over a header chain whose block times equal their heights, so each MTP is predictable. They cover `prev_mtp` and `tip_mtp`, saturation at low heights, and `None` for an output whose transaction is confirmed only transitively, even though the full window is there. Co-Authored-By: Claude Opus 4.8 (1M context) Co-Authored-By: Claude Opus 5 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01NND8BhEmYTYdLRv4dbUUro --- crates/bitcoind_rpc/examples/filter_iter.rs | 2 +- crates/bitcoind_rpc/tests/test_emitter.rs | 6 +- crates/chain/benches/canonicalization.rs | 6 +- crates/chain/benches/indexer.rs | 2 +- crates/chain/benches/trust_classification.rs | 10 +- crates/chain/src/canonical.rs | 128 ++++-- crates/chain/src/canonical_task.rs | 273 ++++++------ crates/chain/src/canonical_view_task.rs | 387 ++++++++++++------ crates/chain/src/indexed_tx_graph.rs | 6 +- crates/chain/src/local_chain.rs | 168 ++++---- crates/chain/src/tx_graph.rs | 6 +- crates/chain/tests/test_canonical_view.rs | 33 +- crates/chain/tests/test_canonical_view_mtp.rs | 181 ++++++++ .../chain/tests/test_canonical_view_task.rs | 12 +- crates/chain/tests/test_indexed_tx_graph.rs | 4 +- crates/chain/tests/test_tx_graph.rs | 10 +- crates/chain/tests/test_tx_graph_conflicts.rs | 6 +- crates/core/src/block_queries.rs | 67 +++ crates/core/src/chain_query.rs | 41 -- crates/core/src/chain_task.rs | 132 ++++++ crates/core/src/lib.rs | 7 +- crates/electrum/tests/test_electrum.rs | 6 +- crates/esplora/tests/async_ext.rs | 4 +- crates/esplora/tests/blocking_ext.rs | 4 +- 24 files changed, 1039 insertions(+), 462 deletions(-) create mode 100644 crates/chain/tests/test_canonical_view_mtp.rs create mode 100644 crates/core/src/block_queries.rs delete mode 100644 crates/core/src/chain_query.rs create mode 100644 crates/core/src/chain_task.rs diff --git a/crates/bitcoind_rpc/examples/filter_iter.rs b/crates/bitcoind_rpc/examples/filter_iter.rs index 7719b87c5f..b8c598e040 100644 --- a/crates/bitcoind_rpc/examples/filter_iter.rs +++ b/crates/bitcoind_rpc/examples/filter_iter.rs @@ -69,7 +69,7 @@ fn main() -> anyhow::Result<()> { println!("Local tip: {}", chain.tip().height()); let canonical_view = - chain.canonical_view(graph.graph(), chain.tip().block_id(), Default::default()); + chain.canonicalize(graph.graph(), chain.tip().block_id(), Default::default()); let unspent: Vec<_> = canonical_view .filter_unspent_outpoints(graph.index.outpoints().clone()) diff --git a/crates/bitcoind_rpc/tests/test_emitter.rs b/crates/bitcoind_rpc/tests/test_emitter.rs index 81b250909a..ac4a3657c2 100644 --- a/crates/bitcoind_rpc/tests/test_emitter.rs +++ b/crates/bitcoind_rpc/tests/test_emitter.rs @@ -322,7 +322,7 @@ fn get_balance( ) -> anyhow::Result { let outpoints = recv_graph.index.outpoints().clone(); let balance = recv_chain - .canonical_view( + .canonicalize( recv_graph.graph(), recv_chain.tip().block_id(), Default::default(), @@ -642,7 +642,7 @@ fn test_expect_tx_evicted() -> anyhow::Result<()> { // Retrieve the expected unconfirmed txids and spks from the graph. let exp_spk_txids = chain - .canonical_view(graph.graph(), chain_tip, Default::default()) + .canonicalize(graph.graph(), chain_tip, Default::default()) .list_expected_spk_txids(&graph.index, ..) .collect::>(); assert_eq!(exp_spk_txids, vec![(spk, txid_1)]); @@ -658,7 +658,7 @@ fn test_expect_tx_evicted() -> anyhow::Result<()> { let _ = graph.batch_insert_relevant_evicted_at(mempool_event.evicted); let canonical_txids = chain - .canonical_view(graph.graph(), chain_tip, Default::default()) + .canonicalize(graph.graph(), chain_tip, Default::default()) .txs() .map(|tx| tx.txid) .collect::>(); diff --git a/crates/chain/benches/canonicalization.rs b/crates/chain/benches/canonicalization.rs index 74e6c05fd9..a1caa813b3 100644 --- a/crates/chain/benches/canonicalization.rs +++ b/crates/chain/benches/canonicalization.rs @@ -94,19 +94,19 @@ fn setup(f: F) -> (KeychainTxGraph, Lo } fn run_list_canonical_txs(tx_graph: &KeychainTxGraph, chain: &LocalChain, exp_txs: usize) { - let view = chain.canonical_view(tx_graph.graph(), chain.tip().block_id(), Default::default()); + let view = chain.canonicalize(tx_graph.graph(), chain.tip().block_id(), Default::default()); let txs = view.txs(); assert_eq!(txs.count(), exp_txs); } fn run_filter_chain_txouts(tx_graph: &KeychainTxGraph, chain: &LocalChain, exp_txos: usize) { - let view = chain.canonical_view(tx_graph.graph(), chain.tip().block_id(), Default::default()); + let view = chain.canonicalize(tx_graph.graph(), chain.tip().block_id(), Default::default()); let utxos = view.filter_outpoints(tx_graph.index.outpoints().clone()); assert_eq!(utxos.count(), exp_txos); } fn run_filter_chain_unspents(tx_graph: &KeychainTxGraph, chain: &LocalChain, exp_utxos: usize) { - let view = chain.canonical_view(tx_graph.graph(), chain.tip().block_id(), Default::default()); + let view = chain.canonicalize(tx_graph.graph(), chain.tip().block_id(), Default::default()); let utxos = view.filter_unspent_outpoints(tx_graph.index.outpoints().clone()); assert_eq!(utxos.count(), exp_utxos); } diff --git a/crates/chain/benches/indexer.rs b/crates/chain/benches/indexer.rs index 27be1bcbcd..ebc07ffb38 100644 --- a/crates/chain/benches/indexer.rs +++ b/crates/chain/benches/indexer.rs @@ -84,7 +84,7 @@ fn do_bench(indexed_tx_graph: &KeychainTxGraph, chain: &LocalChain) { // Check balance let op = graph.index.outpoints().clone(); let bal = chain - .canonical_view(graph.graph(), chain.tip().block_id(), Default::default()) + .canonicalize(graph.graph(), chain.tip().block_id(), Default::default()) .balance( op.into_iter().map(|(_, o)| o), bdk_chain::taints_unowned(&graph.index), diff --git a/crates/chain/benches/trust_classification.rs b/crates/chain/benches/trust_classification.rs index e4e4674da6..606690b755 100644 --- a/crates/chain/benches/trust_classification.rs +++ b/crates/chain/benches/trust_classification.rs @@ -420,7 +420,7 @@ fn bench_fan_in(c: &mut Criterion) { for (width, depth) in [(10, 3), (50, 5), (100, 10), (500, 20)] { let label = format!("{width}w×{depth}d"); let (chain, tx_graph, utxo_txids, owned) = setup_fan_in(width, depth); - let view = chain.canonical_view(&tx_graph, chain.tip().block_id(), Default::default()); + let view = chain.canonicalize(&tx_graph, chain.tip().block_id(), Default::default()); group.bench_with_input(BenchmarkId::new("classify", &label), &(), |b, _| { b.iter(|| run_classify(&utxo_txids, &view, &owned)); }); @@ -433,7 +433,7 @@ fn bench_disjoint(c: &mut Criterion) { for (width, depth) in [(10, 3), (50, 5), (100, 10), (500, 20)] { let label = format!("{width}w×{depth}d"); let (chain, tx_graph, utxo_txids, owned) = setup_disjoint(width, depth); - let view = chain.canonical_view(&tx_graph, chain.tip().block_id(), Default::default()); + let view = chain.canonicalize(&tx_graph, chain.tip().block_id(), Default::default()); group.bench_with_input(BenchmarkId::new("classify", &label), &(), |b, _| { b.iter(|| run_classify(&utxo_txids, &view, &owned)); }); @@ -446,7 +446,7 @@ fn bench_untrusted_fan_in(c: &mut Criterion) { for (width, depth) in [(10, 3), (50, 5), (100, 10), (500, 20)] { let label = format!("{width}w×{depth}d"); let (chain, tx_graph, utxo_txids, owned) = setup_untrusted_fan_in(width, depth); - let view = chain.canonical_view(&tx_graph, chain.tip().block_id(), Default::default()); + let view = chain.canonicalize(&tx_graph, chain.tip().block_id(), Default::default()); group.bench_with_input(BenchmarkId::new("classify", &label), &(), |b, _| { b.iter(|| run_classify(&utxo_txids, &view, &owned)); }); @@ -459,7 +459,7 @@ fn bench_diamond(c: &mut Criterion) { for (shared, utxos) in [(5, 20), (10, 50), (20, 100), (50, 500)] { let label = format!("{shared}s×{utxos}u"); let (chain, tx_graph, utxo_txids, owned) = setup_diamond(shared, utxos); - let view = chain.canonical_view(&tx_graph, chain.tip().block_id(), Default::default()); + let view = chain.canonicalize(&tx_graph, chain.tip().block_id(), Default::default()); group.bench_with_input(BenchmarkId::new("classify", &label), &(), |b, _| { b.iter(|| run_classify(&utxo_txids, &view, &owned)); }); @@ -472,7 +472,7 @@ fn bench_tainted_midchain(c: &mut Criterion) { for (width, depth) in [(10, 3), (50, 5), (100, 10), (500, 20)] { let label = format!("{width}w×{depth}d"); let (chain, tx_graph, utxo_txids, owned) = setup_tainted_midchain(width, depth); - let view = chain.canonical_view(&tx_graph, chain.tip().block_id(), Default::default()); + let view = chain.canonicalize(&tx_graph, chain.tip().block_id(), Default::default()); group.bench_with_input(BenchmarkId::new("classify", &label), &(), |b, _| { b.iter(|| run_classify(&utxo_txids, &view, &owned)); }); diff --git a/crates/chain/src/canonical.rs b/crates/chain/src/canonical.rs index 2650646291..a0fa612e1a 100644 --- a/crates/chain/src/canonical.rs +++ b/crates/chain/src/canonical.rs @@ -6,15 +6,14 @@ //! ## Example //! //! ``` -//! # use bdk_chain::{TxGraph, CanonicalParams, CanonicalTask, local_chain::LocalChain}; +//! # use bdk_chain::{TxGraph, CanonicalParams, local_chain::LocalChain}; //! # use bdk_core::BlockId; //! # use bitcoin::hashes::Hash; //! # let tx_graph = TxGraph::::default(); //! # let chain = LocalChain::from_blocks([(0, bitcoin::BlockHash::all_zeros())].into_iter().collect()).unwrap(); //! let chain_tip = chain.tip().block_id(); //! let params = CanonicalParams::default(); -//! let task = CanonicalTask::new(&tx_graph, chain_tip, params); -//! let view = chain.canonicalize(task); +//! let view = chain.canonicalize(&tx_graph, chain_tip, params); //! //! // Iterate over canonical transactions //! for tx in view.txs() { @@ -27,7 +26,7 @@ use alloc::sync::Arc; use alloc::vec::Vec; use core::{fmt, ops::RangeBounds}; -use bdk_core::BlockId; +use bdk_core::{BlockId, BlockQueries}; use bitcoin::{constants::COINBASE_MATURITY, OutPoint, ScriptBuf, Transaction, TxOut, Txid}; use crate::{spk_txout::SpkTxOutIndex, Anchor, Balance, CanonicalViewTask, ChainPosition, TxGraph}; @@ -60,6 +59,14 @@ pub enum Trust { Unknown, } +/// Internal per-transaction entry in [`Canonical`]. +#[derive(Clone, Debug)] +pub(crate) struct CanonicalEntry

{ + pub(crate) tx: Arc, + pub(crate) pos: P, + pub(crate) prev_mtp: Option, +} + /// A single canonical transaction with its position. /// /// This struct represents a transaction that has been determined to be canonical (not @@ -112,6 +119,25 @@ pub struct CanonicalTxOut

{ pub spent_by: Option<(P, Txid)>, /// Whether this output is on a coinbase transaction. pub is_on_coinbase: bool, + /// 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. + /// + /// `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 output's transaction is not confirmed. + /// - The output's transaction is confirmed only *transitively*, through a descendant's anchor, + /// so 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 output can have `None` here even with MTP enabled. Do not read `None` as + /// "MTP was off", and never treat it as an absent or satisfied timelock. + pub prev_mtp: Option, } impl Ord for CanonicalTxOut

{ @@ -205,12 +231,14 @@ impl CanonicalTxOut> { pub struct Canonical { /// List of canonical transaction IDs. pub(crate) order: Vec, - /// Map of transaction IDs to their transaction data and position. - pub(crate) txs: HashMap, P)>, + /// Map of transaction IDs to their transaction data, position, and MTP. + pub(crate) txs: HashMap>, /// Map of outpoints to the transaction ID that spends them. pub(crate) spends: HashMap, /// The chain tip at the time this view was created. pub(crate) tip: BlockId, + /// Median-time-past at the chain tip height. + pub(crate) tip_mtp: Option, /// Marker for the anchor type. pub(crate) _anchor: core::marker::PhantomData, } @@ -232,14 +260,16 @@ impl Canonical { pub(crate) fn new( tip: BlockId, order: Vec, - txs: HashMap, P)>, + txs: HashMap>, spends: HashMap, + tip_mtp: Option, ) -> Self { Self { tip, order, txs, spends, + tip_mtp, _anchor: core::marker::PhantomData, } } @@ -249,15 +279,24 @@ impl Canonical { self.tip } + /// Get the MTP at the chain tip height. + /// + /// Returns `None` if MTP was not computed. + pub fn tip_mtp(&self) -> Option { + self.tip_mtp + } + /// Get a single canonical transaction by its transaction ID. /// /// Returns `Some(CanonicalTx)` if the transaction exists in the canonical set, /// or `None` if the transaction doesn't exist or was excluded due to conflicts. pub fn tx(&self, txid: Txid) -> Option> { - self.txs - .get(&txid) - .cloned() - .map(|(tx, pos)| CanonicalTx { pos, txid, tx }) + let entry = self.txs.get(&txid)?; + Some(CanonicalTx { + pos: entry.pos.clone(), + txid, + tx: entry.tx.clone(), + }) } /// Get a single canonical transaction output. @@ -270,19 +309,20 @@ impl Canonical { /// - The output index is out of bounds /// - The transaction was excluded due to conflicts pub fn txout(&self, op: OutPoint) -> Option> { - let (tx, pos) = self.txs.get(&op.txid)?; + let entry = self.txs.get(&op.txid)?; let vout: usize = op.vout.try_into().ok()?; - let txout = tx.output.get(vout)?; + let txout = entry.tx.output.get(vout)?; let spent_by = self.spends.get(&op).map(|spent_by_txid| { - let (_, spent_by_pos) = &self.txs[spent_by_txid]; - (spent_by_pos.clone(), *spent_by_txid) + let spent_by_entry = &self.txs[spent_by_txid]; + (spent_by_entry.pos.clone(), *spent_by_txid) }); Some(CanonicalTxOut { - pos: pos.clone(), + pos: entry.pos.clone(), outpoint: op, txout: txout.clone(), spent_by, - is_on_coinbase: tx.is_coinbase(), + is_on_coinbase: entry.tx.is_coinbase(), + prev_mtp: entry.prev_mtp, }) } @@ -294,14 +334,13 @@ impl Canonical { /// # Example /// /// ``` - /// # use bdk_chain::{TxGraph, CanonicalTask, local_chain::LocalChain}; + /// # use bdk_chain::{TxGraph, local_chain::LocalChain}; /// # use bdk_core::BlockId; /// # use bitcoin::hashes::Hash; /// # let tx_graph = TxGraph::::default(); /// # let chain = LocalChain::from_blocks([(0, bitcoin::BlockHash::all_zeros())].into_iter().collect()).unwrap(); /// # let chain_tip = chain.tip().block_id(); - /// # let task = CanonicalTask::new(&tx_graph, chain_tip, Default::default()); - /// # let view = chain.canonicalize(task); + /// # let view = chain.canonicalize(&tx_graph, chain_tip, Default::default()); /// // Iterate over all canonical transactions /// for tx in view.txs() { /// println!("TX {}: {:?}", tx.txid, tx.pos); @@ -312,8 +351,12 @@ impl Canonical { /// ``` pub fn txs(&self) -> impl ExactSizeIterator> + DoubleEndedIterator + '_ { self.order.iter().map(|&txid| { - let (tx, pos) = self.txs[&txid].clone(); - CanonicalTx { pos, txid, tx } + let entry = &self.txs[&txid]; + CanonicalTx { + pos: entry.pos.clone(), + txid, + tx: entry.tx.clone(), + } }) } @@ -329,14 +372,13 @@ impl Canonical { /// # Example /// /// ``` - /// # use bdk_chain::{TxGraph, CanonicalTask, local_chain::LocalChain, keychain_txout::KeychainTxOutIndex}; + /// # use bdk_chain::{TxGraph, local_chain::LocalChain, keychain_txout::KeychainTxOutIndex}; /// # use bdk_core::BlockId; /// # use bitcoin::hashes::Hash; /// # let tx_graph = TxGraph::::default(); /// # let chain = LocalChain::from_blocks([(0, bitcoin::BlockHash::all_zeros())].into_iter().collect()).unwrap(); /// # let chain_tip = chain.tip().block_id(); - /// # let task = CanonicalTask::new(&tx_graph, chain_tip, Default::default()); - /// # let view = chain.canonicalize(task); + /// # let view = chain.canonicalize(&tx_graph, chain_tip, Default::default()); /// # let indexer = KeychainTxOutIndex::<&str>::default(); /// // Get all outputs from an indexer /// for (keychain, txout) in view.filter_outpoints(indexer.outpoints().clone()) { @@ -360,14 +402,13 @@ impl Canonical { /// # Example /// /// ``` - /// # use bdk_chain::{TxGraph, CanonicalTask, local_chain::LocalChain, keychain_txout::KeychainTxOutIndex}; + /// # use bdk_chain::{TxGraph, local_chain::LocalChain, keychain_txout::KeychainTxOutIndex}; /// # use bdk_core::BlockId; /// # use bitcoin::hashes::Hash; /// # let tx_graph = TxGraph::::default(); /// # let chain = LocalChain::from_blocks([(0, bitcoin::BlockHash::all_zeros())].into_iter().collect()).unwrap(); /// # let chain_tip = chain.tip().block_id(); - /// # let task = CanonicalTask::new(&tx_graph, chain_tip, Default::default()); - /// # let view = chain.canonicalize(task); + /// # let view = chain.canonicalize(&tx_graph, chain_tip, Default::default()); /// # let indexer = KeychainTxOutIndex::<&str>::default(); /// // Get unspent outputs (UTXOs) from an indexer /// for (keychain, utxo) in view.filter_unspent_outpoints(indexer.outpoints().clone()) { @@ -585,7 +626,7 @@ impl CanonicalView { /// # let tx_graph = TxGraph::::default(); /// # let chain = LocalChain::from_blocks([(0, bitcoin::BlockHash::all_zeros())].into_iter().collect()).unwrap(); /// # let chain_tip = chain.tip().block_id(); - /// # let view = chain.canonical_view(&tx_graph, chain_tip, CanonicalParams::default()); + /// # let view = chain.canonicalize(&tx_graph, chain_tip, CanonicalParams::default()); /// # let indexer = KeychainTxOutIndex::<&str>::default(); /// let tip_height = view.tip().height; /// // Calculate balance requiring 6 confirmations. @@ -646,13 +687,26 @@ where } impl CanonicalTxs { - /// Creates a [`CanonicalViewTask`] that resolves [`CanonicalReason`](crate::CanonicalReason)s - /// into [`ChainPosition`]s. - /// - /// This is the second phase of the canonicalization pipeline. The resulting task - /// queries the chain to verify anchors for transitively anchored transactions and - /// produces a [`CanonicalView`] with resolved chain positions. - pub fn view_task<'g>(self, tx_graph: &'g TxGraph) -> CanonicalViewTask<'g, A> { - CanonicalViewTask::new(tx_graph, self.tip, self.order, self.txs, self.spends) + /// Creates a [`CanonicalViewTask`] that resolves + /// [`CanonicalReason`](crate::canonical_task::CanonicalReason)s into [`ChainPosition`]s. + /// + /// This is the second phase of the canonicalization pipeline. Blocks fetched during + /// phase 1 are passed through so they can be reused without redundant queries. + /// + /// To also compute median-time-past (MTP) values for the resulting view, call + /// [`CanonicalViewTask::with_mtp`] on the returned task. + pub fn view_task<'g, B>( + self, + tx_graph: &'g TxGraph, + queries: BlockQueries, + ) -> CanonicalViewTask<'g, A, B> { + CanonicalViewTask::new( + tx_graph, + self.tip, + self.order, + self.txs, + self.spends, + queries, + ) } } diff --git a/crates/chain/src/canonical_task.rs b/crates/chain/src/canonical_task.rs index 693a3229bb..a8e239045b 100644 --- a/crates/chain/src/canonical_task.rs +++ b/crates/chain/src/canonical_task.rs @@ -1,14 +1,14 @@ use crate::collections::{HashMap, HashSet, VecDeque}; use crate::tx_graph::{TxAncestors, TxDescendants}; -use crate::{Anchor, CanonicalTxs, TxGraph}; +use crate::{canonical::CanonicalEntry, Anchor, CanonicalTxs, TxGraph}; use alloc::boxed::Box; use alloc::collections::BTreeSet; use alloc::sync::Arc; use alloc::vec::Vec; -use bdk_core::{BlockId, ChainQuery, ChainRequest, ChainResponse}; +use bdk_core::{BlockId, BlockQueries, ChainTask, TaskProgress, ToBlockHash}; use bitcoin::{Transaction, Txid}; -type CanonicalMap = HashMap, CanonicalReason)>; +type CanonicalMap = HashMap>>; type NotCanonicalSet = HashSet; /// Represents the current stage of canonicalization processing. @@ -28,7 +28,7 @@ enum CanonicalStage { } impl CanonicalStage { - fn advance(&mut self) { + fn next_stage(&mut self) { *self = match self { CanonicalStage::AssumedTxs => Self::AnchoredTxs, CanonicalStage::AnchoredTxs => Self::SeenTxs, @@ -56,10 +56,12 @@ pub struct CanonicalParams { /// (via [`CanonicalReason`](crate::CanonicalReason)). The output is a [`CanonicalTxs`] which can /// then be further processed by [`CanonicalViewTask`](crate::CanonicalViewTask) to resolve reasons /// into [`ChainPosition`](crate::ChainPosition)s. -pub struct CanonicalTask<'g, A> { +pub struct CanonicalTask<'g, A, B> { tx_graph: &'g TxGraph, chain_tip: BlockId, + queries: BlockQueries, + unprocessed_assumed_txs: Box)> + 'g>, unprocessed_anchored_txs: VecDeque<(Txid, Arc, &'g BTreeSet)>, unprocessed_seen_txs: Box, u64)> + 'g>, @@ -75,153 +77,176 @@ pub struct CanonicalTask<'g, A> { current_stage: CanonicalStage, } -impl<'g, A: Anchor> ChainQuery for CanonicalTask<'g, A> { - type Output = CanonicalTxs; +impl<'g, A: Anchor, B: ToBlockHash> ChainTask for CanonicalTask<'g, A, B> { + type Output = (CanonicalTxs, BlockQueries); fn tip(&self) -> BlockId { self.chain_tip } - fn next_query(&mut self) -> Option { - loop { - match self.current_stage { - CanonicalStage::AssumedTxs => { - if let Some((txid, tx)) = self.unprocessed_assumed_txs.next() { - if !self.is_canonicalized(txid) { - self.mark_canonical(txid, tx, CanonicalReason::assumed()); - } - continue; + fn unresolved_queries<'a>(&'a self) -> impl Iterator + 'a { + self.queries.unresolved() + } + + fn poll(&mut self) -> TaskProgress { + match self.current_stage { + CanonicalStage::AssumedTxs => { + if let Some((txid, tx)) = self.unprocessed_assumed_txs.next() { + if !self.is_canonicalized(txid) { + self.mark_canonical(txid, tx, CanonicalReason::assumed()); } + } else { + self.current_stage.next_stage(); } - CanonicalStage::AnchoredTxs => { - if let Some((_txid, _, anchors)) = self.unprocessed_anchored_txs.front() { - let block_ids = - anchors.iter().map(|anchor| anchor.anchor_block()).collect(); - return Some(block_ids); + TaskProgress::Advanced + } + CanonicalStage::AnchoredTxs => { + // We query block data lazily, one frontier tx at a time, rather than batching all + // anchor heights up front. This is deliberate: anchored txs are processed + // latest-first, and marking one canonical sweeps all its ancestors transitively + // (see `mark_canonical`) without any further queries. So the number of queries is + // bounded by the *frontier* of each confirmed subgraph, not the total tx count. + // Eager batching would over-query heights for ancestors that transitivity covers + // for free. + // + // The trade-off: a driver where each query is a network round-trip (e.g. a future + // Bitcoin Core RPC driver) sees one serial round-trip per frontier tx, instead of + // a hashmap lookup. Batching these is tracked in #2403. + if let Some((txid, tx, anchors)) = self.unprocessed_anchored_txs.pop_front() { + if self.is_canonicalized(txid) { + return TaskProgress::Advanced; } - } - CanonicalStage::SeenTxs => { - if let Some((txid, tx, last_seen)) = self.unprocessed_seen_txs.next() { - // A coinbase tx cannot be in the mempool, so a `last_seen` value on one is - // not evidence of it being canonical. - if !self.is_canonicalized(txid) && !tx.is_coinbase() { - let observed_in = ObservedIn::Mempool(last_seen); - self.mark_canonical( - txid, - tx, - CanonicalReason::from_observed_in(observed_in), - ); + + let mut best_anchor = Option::::None; + for a in anchors.iter() { + let h = a.anchor_block().height; + // A block that is either unresolved (`None`), absent at this height + // (`Some(None)`), or present but mismatched does not confirm this anchor. + if let Some(Some(b)) = self.queries.get(h) { + if b.to_blockhash() == a.anchor_block().hash { + best_anchor = Some(a.clone()); + break; + } } - continue; } - } - CanonicalStage::LeftOverTxs => { - if let Some((txid, tx, height)) = self.unprocessed_leftover_txs.pop_front() { - let is_evicted = self - .tx_graph - .get_tx_node(txid) - .expect("leftover transaction must exist") - .is_evicted(); - if !self.is_canonicalized(txid) && !tx.is_coinbase() && !is_evicted { - let observed_in = ObservedIn::Block(height); - self.mark_canonical( - txid, - tx, - CanonicalReason::from_observed_in(observed_in), - ); - } - continue; + + if let Some(a) = best_anchor { + self.mark_canonical(txid, tx, CanonicalReason::from_anchor(a)); + return TaskProgress::Advanced; } - } - CanonicalStage::Finished => return None, - } - self.current_stage.advance(); - } - } + // No resolved anchor matched. `request` returns only heights that are neither + // resolved nor already in-flight, so a non-empty result is genuinely new work + // to fetch. This keeps `Query` additive and never empty. + let heights = self + .queries + .request(anchors.iter().map(|a| a.anchor_block().height)); + if !heights.is_empty() { + self.unprocessed_anchored_txs + .push_front((txid, tx, anchors)); + return TaskProgress::Query(heights); + } - fn resolve_query(&mut self, response: ChainResponse) { - match self.current_stage { - CanonicalStage::AnchoredTxs => { - // Process directly anchored transaction response - if let Some((txid, tx, anchors)) = self.unprocessed_anchored_txs.pop_front() { - // Find the anchor that matches the confirmed BlockId - let best_anchor = response.and_then(|block_id| { + // No new heights to request. If any anchor height is still in-flight (requested + // but unresolved), we can't decide this tx yet — wait for the driver. Otherwise + // every anchor height is resolved and none matched, so the tx is not confirmed. + if anchors + .iter() + .any(|a| self.queries.get(a.anchor_block().height).is_none()) + { + self.unprocessed_anchored_txs + .push_front((txid, tx, anchors)); + return TaskProgress::Blocked; + } + + // No confirmed anchor found. + self.unprocessed_leftover_txs.push_back(( + txid, + tx, anchors .iter() - .find(|anchor| anchor.anchor_block() == block_id) - .cloned() - }); - - match best_anchor { - Some(best_anchor) => { - // Transaction has a confirmed anchor - if !self.is_canonicalized(txid) { - self.mark_canonical( - txid, - tx, - CanonicalReason::from_anchor(best_anchor), - ); - } - } - None => { - // No confirmed anchor found, add to leftover transactions for later - // processing - self.unprocessed_leftover_txs.push_back(( - txid, - tx, - anchors - .iter() - .last() - .expect( - "tx taken from `unprocessed_anchored_txs` so it must have at least one anchor", - ) - .anchor_block() - .height, - )) - } + .last() + .expect("must have at least one anchor") + .anchor_block() + .height, + )); + } else { + self.current_stage.next_stage(); + } + TaskProgress::Advanced + } + CanonicalStage::SeenTxs => { + if let Some((txid, tx, last_seen)) = self.unprocessed_seen_txs.next() { + // A coinbase tx cannot be in the mempool, so a `last_seen` value on one is + // not evidence of it being canonical. + if !self.is_canonicalized(txid) && !tx.is_coinbase() { + let observed_in = ObservedIn::Mempool(last_seen); + self.mark_canonical( + txid, + tx, + CanonicalReason::from_observed_in(observed_in), + ); } + } else { + self.current_stage.next_stage(); } + TaskProgress::Advanced } - CanonicalStage::AssumedTxs - | CanonicalStage::SeenTxs - | CanonicalStage::LeftOverTxs - | CanonicalStage::Finished => { - // These stages don't generate queries and shouldn't receive responses - debug_assert!( - false, - "resolve_query called for stage {:?} which doesn't generate queries", - self.current_stage - ); + CanonicalStage::LeftOverTxs => { + if let Some((txid, tx, height)) = self.unprocessed_leftover_txs.pop_front() { + let is_evicted = self + .tx_graph + .get_tx_node(txid) + .expect("leftover transaction must exist") + .is_evicted(); + if !self.is_canonicalized(txid) && !tx.is_coinbase() && !is_evicted { + let observed_in = ObservedIn::Block(height); + self.mark_canonical( + txid, + tx, + CanonicalReason::from_observed_in(observed_in), + ); + } + } else { + self.current_stage.next_stage(); + } + TaskProgress::Advanced } + CanonicalStage::Finished => TaskProgress::Done, } } + fn resolve_query(&mut self, height: u32, block: Option) { + self.queries.resolve(height, block); + } + fn finish(self) -> Self::Output { - let mut view_order = Vec::new(); - let mut view_txs = HashMap::new(); let mut view_spends = HashMap::new(); for txid in &self.canonical_order { - if let Some((tx, reason)) = self.canonical.get(txid) { - view_order.push(*txid); - - // Add spends - if !tx.is_coinbase() { - for input in &tx.input { + if let Some(entry) = self.canonical.get(txid) { + if !entry.tx.is_coinbase() { + for input in &entry.tx.input { view_spends.insert(input.previous_output, *txid); } } - - view_txs.insert(*txid, (tx.clone(), reason.clone())); } } - CanonicalTxs::new(self.chain_tip, view_order, view_txs, view_spends) + ( + CanonicalTxs::new( + self.chain_tip, + self.canonical_order, + self.canonical, + view_spends, + None, + ), + self.queries, + ) } } -impl<'g, A: Anchor> CanonicalTask<'g, A> { +impl<'g, A: Anchor, B: ToBlockHash> CanonicalTask<'g, A, B> { /// Creates a new canonicalization task. pub fn new(tx_graph: &'g TxGraph, chain_tip: BlockId, params: CanonicalParams) -> Self { let anchors = tx_graph.all_anchors(); @@ -256,6 +281,8 @@ impl<'g, A: Anchor> CanonicalTask<'g, A> { canonical_order: Vec::new(), current_stage: CanonicalStage::default(), + + queries: BlockQueries::new(), } } @@ -321,7 +348,11 @@ impl<'g, A: Anchor> CanonicalTask<'g, A> { } staged_canonical.push((this_txid, tx.clone(), this_reason.clone())); - canonical_entry.insert((tx.clone(), this_reason)); + canonical_entry.insert(CanonicalEntry { + tx: tx.clone(), + pos: this_reason, + prev_mtp: None, + }); Some(this_txid) }, ) @@ -490,9 +521,9 @@ mod tests { // Create canonicalization task and canonicalize using the two-step pipeline let params = CanonicalParams::default(); let task = CanonicalTask::new(&tx_graph, chain_tip, params); - let canonical_txs = chain.canonicalize(task); - let view_task = canonical_txs.view_task(&tx_graph); - let canonical_view = chain.canonicalize(view_task); + let (txs, queries) = chain.run_task(task); + let view_task = txs.view_task(&tx_graph, queries); + let canonical_view = chain.run_task(view_task); // Should have one canonical transaction assert_eq!(canonical_view.txs().len(), 1); diff --git a/crates/chain/src/canonical_view_task.rs b/crates/chain/src/canonical_view_task.rs index d981bcef44..b215c3f894 100644 --- a/crates/chain/src/canonical_view_task.rs +++ b/crates/chain/src/canonical_view_task.rs @@ -4,20 +4,21 @@ use crate::canonical_task::{CanonicalReason, ObservedIn}; use crate::collections::{HashMap, VecDeque}; use crate::tx_graph::TxDescendants; use alloc::collections::BTreeSet; -use alloc::sync::Arc; use alloc::vec::Vec; -use bdk_core::{BlockId, ChainQuery, ChainRequest, ChainResponse}; -use bitcoin::{OutPoint, Transaction, Txid}; +use bdk_core::{BlockId, BlockQueries, ChainTask, TaskProgress, ToBlockHash, ToBlockTime}; +use bitcoin::{OutPoint, Txid}; -use crate::{Anchor, CanonicalView, ChainPosition, TxGraph}; +use crate::{canonical::CanonicalEntry, Anchor, CanonicalView, ChainPosition, TxGraph}; /// Represents the current stage of view task processing. #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] enum ViewStage { - /// Processing transactions to resolve their chain positions. + /// Verifying anchors for transitively anchored transactions. #[default] ResolvingPositions, + /// Fetching blocks needed for MTP computation. + FetchingMtpBlocks, /// All processing is complete. Finished, } @@ -29,195 +30,333 @@ enum ViewStage { /// reason into a concrete [`ChainPosition`] (confirmed or unconfirmed). For transitively /// anchored transactions, it queries the chain to check if they have their own direct /// anchors. -pub struct CanonicalViewTask<'g, A> { +/// +/// When `with_mtp()` is called, this task also computes median-time-past (MTP) values +/// for confirmed heights and stores them in the resulting [`CanonicalView`]. +pub struct CanonicalViewTask<'g, A, B> { tx_graph: &'g TxGraph, tip: BlockId, - /// Transactions in canonical order with their reasons. + queries: BlockQueries, + canonical_order: Vec, - canonical_txs: HashMap, CanonicalReason)>, + canonical_txs: HashMap>>, spends: HashMap, - - /// Transactions that need anchor verification (transitively anchored). unprocessed_anchor_checks: VecDeque<(Txid, &'g BTreeSet)>, - - /// Resolved direct anchors for transitively anchored transactions. direct_anchors: HashMap, + // MTP support — `extract_time` being `Some` means MTP is enabled. + extract_time: Option u32>, + current_stage: ViewStage, } -impl<'g, A: Anchor> CanonicalViewTask<'g, A> { +impl<'g, A: Anchor, B> CanonicalViewTask<'g, A, B> { /// Creates a new [`CanonicalViewTask`]. /// - /// Accepts canonical transaction data and a reference to the [`TxGraph`]. - /// Scans transactions to find those needing anchor verification. - pub fn new( + /// Accepts canonical transaction data, a reference to the [`TxGraph`], and blocks + /// already fetched during phase 1 (to avoid redundant queries). + pub(crate) fn new( tx_graph: &'g TxGraph, tip: BlockId, order: Vec, - txs: HashMap, CanonicalReason)>, + txs: HashMap>>, spends: HashMap, + queries: BlockQueries, ) -> Self { let all_anchors = tx_graph.all_anchors(); let mut unprocessed_anchor_checks = VecDeque::new(); + let mut direct_anchors = HashMap::new(); for txid in &order { - if let Some((_, reason)) = txs.get(txid) { - if matches!(reason, CanonicalReason::ObservedIn { .. }) { + // `mark_canonical` inserts into the canonical map and pushes to the canonical order + // in the same call, and only ever removes from the map before that push, so every + // txid in `order` has an entry in `txs`. + let entry = match txs.get(txid) { + Some(entry) => entry, + None => { + debug_assert!(false, "every txid in `order` must have an entry in `txs`"); continue; } - if reason.is_transitive() || reason.is_assumed() { + }; + match &entry.pos { + CanonicalReason::Anchor { + anchor, + descendant: None, + } => { + // Non-transitive anchor — already resolved. + direct_anchors.insert(*txid, anchor.clone()); + } + CanonicalReason::Anchor { .. } | CanonicalReason::Assumed { .. } => { + // Transitive or assumed — needs anchor verification. The tx may have no + // anchors of its own: a transitive reason carries the *descendant's* anchor, + // and an assumed one needs no chain evidence at all. if let Some(anchors) = all_anchors.get(txid) { unprocessed_anchor_checks.push_back((*txid, anchors)); } } + CanonicalReason::ObservedIn { .. } => {} } } Self { tx_graph, tip, + queries, canonical_order: order, canonical_txs: txs, spends, unprocessed_anchor_checks, - direct_anchors: HashMap::new(), + direct_anchors, + extract_time: None, current_stage: ViewStage::default(), } } } -impl<'g, A: Anchor> ChainQuery for CanonicalViewTask<'g, A> { +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 tx it computes the MTP of the block *preceding* the confirmation + /// height (per BIP-68, matching Bitcoin Core's `CalculateSequenceLocks`), exposed on + /// [`CanonicalTxOut::prev_mtp`](crate::CanonicalTxOut::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 + } +} + +impl<'g, A: Anchor, B: ToBlockHash> ChainTask for CanonicalViewTask<'g, A, B> { type Output = CanonicalView; fn tip(&self) -> BlockId { self.tip } - fn next_query(&mut self) -> Option { - loop { - match self.current_stage { - ViewStage::ResolvingPositions => { - if let Some((_txid, anchors)) = self.unprocessed_anchor_checks.front() { - let block_ids = - anchors.iter().map(|anchor| anchor.anchor_block()).collect(); - return Some(block_ids); - } - } - ViewStage::Finished => return None, - } - - self.current_stage = ViewStage::Finished; - } + fn unresolved_queries<'a>(&'a self) -> impl Iterator + 'a { + self.queries.unresolved() } - fn resolve_query(&mut self, response: ChainResponse) { + fn poll(&mut self) -> TaskProgress { match self.current_stage { ViewStage::ResolvingPositions => { if let Some((txid, anchors)) = self.unprocessed_anchor_checks.pop_front() { - let best_anchor = response.and_then(|block_id| { - anchors - .iter() - .find(|anchor| anchor.anchor_block() == block_id) - .cloned() - }); - - if let Some(best_anchor) = best_anchor { - self.direct_anchors.insert(txid, best_anchor); + let mut best_anchor = Option::::None; + for a in anchors.iter() { + let h = a.anchor_block().height; + // Only a resolved, present, matching block confirms this anchor. + if let Some(Some(b)) = self.queries.get(h) { + if b.to_blockhash() == a.anchor_block().hash { + best_anchor = Some(a.clone()); + break; + } + } + } + + if let Some(anchor) = best_anchor { + self.direct_anchors.insert(txid, anchor); + return TaskProgress::Advanced; + } + + // No resolved anchor matched. `request` is additive, so a non-empty result is + // genuinely new work to fetch and is never empty. + let heights = self + .queries + .request(anchors.iter().map(|a| a.anchor_block().height)); + if !heights.is_empty() { + self.unprocessed_anchor_checks.push_front((txid, anchors)); + return TaskProgress::Query(heights); + } + + // No new heights. If any anchor height is still in-flight, wait for the driver; + // otherwise all are resolved and none matched (no direct anchor for this tx). + if anchors + .iter() + .any(|a| self.queries.get(a.anchor_block().height).is_none()) + { + self.unprocessed_anchor_checks.push_front((txid, anchors)); + return TaskProgress::Blocked; } + + // No confirmed anchor found for this tx + TaskProgress::Advanced + } else { + self.current_stage = ViewStage::FetchingMtpBlocks; + TaskProgress::Advanced } } - ViewStage::Finished => { - debug_assert!(false, "resolve_query called in Finished stage"); + ViewStage::FetchingMtpBlocks => { + if self.extract_time.is_none() { + self.current_stage = ViewStage::Finished; + return TaskProgress::Advanced; + } + + // 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 take + // `confirmation_height - 1` (saturating at genesis) — the same value read in + // `finish`. + // + // A `BTreeSet` rather than a `Vec`: the windows overlap heavily (two txs in one + // block give identical windows, adjacent blocks share 10 of 11 heights), and this + // is rebuilt on every poll of this stage. It also makes the emitted `Query` + // ascending and deterministic — `direct_anchors` is a `HashMap`, so iterating its + // values would otherwise vary the height order between runs. + let required: BTreeSet = core::iter::once(self.tip.height) + .chain( + self.direct_anchors + .values() + .map(|a| a.anchor_block().height.saturating_sub(1)), + ) + .flat_map(|h| h.saturating_sub(10)..=h) + .collect(); + + // `request` is additive: a non-empty result is new work to fetch. + let needed = self.queries.request(required.iter().copied()); + if !needed.is_empty() { + return TaskProgress::Query(needed); + } + + // No new heights. If any required height is still in-flight, wait for the driver; + // otherwise every MTP block is resolved and we can finish. + if required.iter().any(|h| self.queries.get(*h).is_none()) { + return TaskProgress::Blocked; + } + + self.current_stage = ViewStage::Finished; + TaskProgress::Advanced } + ViewStage::Finished => TaskProgress::Done, } } + fn resolve_query(&mut self, height: u32, block: Option) { + self.queries.resolve(height, block); + } + fn finish(self) -> Self::Output { + // Helper: compute MTP for a given height from the blocks map. Returns `None` when MTP + // was not enabled, so callers do not each repeat that check. + let try_compute_mtp_at = |h: u32| -> Option { + let extract_time = self.extract_time?; + let start = h.saturating_sub(10); + let mut ts: Vec = (start..=h) + .map(|mtp_h| { + self.queries + .get(mtp_h) + .and_then(|b| b.as_ref()) + .map(extract_time) + }) + .collect::>>()?; + ts.sort_unstable(); + Some(ts[ts.len() / 2]) + }; + + // Compute tip MTP. + let tip_mtp = try_compute_mtp_at(self.tip.height); + let mut view_order = Vec::new(); - let mut view_txs = HashMap::new(); + let mut view_txs = HashMap::>::new(); for txid in &self.canonical_order { - if let Some((tx, reason)) = self.canonical_txs.get(txid) { - view_order.push(*txid); - - // Get transaction node for first_seen/last_seen info - let tx_node = match self.tx_graph.get_tx_node(*txid) { - Some(tx_node) => tx_node, - None => { - debug_assert!(false, "tx node must exist!"); - continue; - } + let (tx, reason) = match self.canonical_txs.get(txid) { + Some(CanonicalEntry { tx, pos, .. }) => (tx, pos), + None => { + debug_assert!(false, "missing entry in `canonical_txs`"); + continue; + } + }; + + view_order.push(*txid); + + // Get transaction node for first_seen/last_seen info. + let tx_node = match self.tx_graph.get_tx_node(*txid) { + Some(tx_node) => tx_node, + None => { + debug_assert!(false, "tx node must exist!"); + continue; + } + }; + + // Handle directly anchored txs first. + if let Some(anchor) = self.direct_anchors.get(txid) { + let tx = tx.clone(); + let pos = ChainPosition::Confirmed { + anchor: anchor.clone(), + transitively: None, }; + // 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 = try_compute_mtp_at(anchor.anchor_block().height.saturating_sub(1)); + view_txs.insert(*txid, CanonicalEntry { tx, pos, prev_mtp }); + continue; + } - // Determine chain position based on reason - let chain_position = match reason { - CanonicalReason::Assumed { .. } => { - match self.direct_anchors.get(txid) { - // it has a direct anchor found - // regardless if it's directly or transitively assumed canonical - Some(anchor) => ChainPosition::Confirmed { - anchor, - transitively: None, - }, - None => TxDescendants::new_exclude_root( - self.tx_graph, - *txid, - // ensure descendant is canonical - |_, desc_txid| -> Option { - self.canonical_txs - .contains_key(&desc_txid) - .then_some(desc_txid) - }, - ) - // ensure descendant has direct anchor - .find_map(|desc_txid| { - self.direct_anchors.get(&desc_txid).map(|anchor| { - ChainPosition::Confirmed { - anchor, - transitively: Some(desc_txid), - } - }) - }) - .unwrap_or(ChainPosition::Unconfirmed { - first_seen: tx_node.first_seen, - last_seen: tx_node.last_seen, - }), - } - } - CanonicalReason::Anchor { anchor, descendant } => match descendant { - Some(_) => match self.direct_anchors.get(txid) { - Some(anchor) => ChainPosition::Confirmed { - anchor, - transitively: None, - }, - None => ChainPosition::Confirmed { - anchor, - transitively: *descendant, - }, - }, - None => ChainPosition::Confirmed { - anchor, - transitively: None, - }, - }, - CanonicalReason::ObservedIn { observed_in, .. } => match observed_in { - ObservedIn::Mempool(last_seen) => ChainPosition::Unconfirmed { - first_seen: tx_node.first_seen, - last_seen: Some(*last_seen), - }, - ObservedIn::Block(_) => ChainPosition::Unconfirmed { - first_seen: tx_node.first_seen, - last_seen: None, + let pos = match reason { + // Since the canonicalization algorithm processes assumed txs first (and therefore + // all their ancestors), some assumed txs may actually be confirmed in the best + // chain. Therefore we need to check all assumed txs if themselves, or any + // descendant of theirs, is anchored in the best chain. + CanonicalReason::Assumed { .. } => { + TxDescendants::new_include_root( + self.tx_graph, + *txid, + // Only explore canonical descendants. + |_, desc_txid| -> Option { + self.canonical_txs + .contains_key(&desc_txid) + .then_some(desc_txid) }, + ) + // Find the earliest instance of an anchored tx. + .find_map(|desc_txid| { + self.direct_anchors + .get(&desc_txid) + .map(|anchor| ChainPosition::Confirmed { + anchor: anchor.clone(), + transitively: Some(desc_txid), + }) + }) + .unwrap_or(ChainPosition::Unconfirmed { + first_seen: tx_node.first_seen, + last_seen: tx_node.last_seen, + }) + } + CanonicalReason::Anchor { anchor, descendant } => ChainPosition::Confirmed { + anchor: anchor.clone(), + transitively: *descendant, + }, + CanonicalReason::ObservedIn { observed_in, .. } => ChainPosition::Unconfirmed { + first_seen: tx_node.first_seen, + // `last_seen` is a mempool sighting. `ObservedIn::Block` means the tx was + // only ever seen in a block that is not in the best chain, so there is no + // mempool sighting to report — a tx that re-entered the mempool would be + // `ObservedIn::Mempool` instead. + last_seen: match observed_in { + ObservedIn::Block(_) => None, + ObservedIn::Mempool(seen_at) => Some(*seen_at), }, - }; - - view_txs.insert(*txid, (tx.clone(), chain_position.cloned())); - } + }, + }; + view_txs.insert( + *txid, + CanonicalEntry { + tx: tx.clone(), + pos, + // 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, + }, + ); } - CanonicalView::new(self.tip, view_order, view_txs, self.spends) + CanonicalView::new(self.tip, view_order, view_txs, self.spends, tip_mtp) } } diff --git a/crates/chain/src/indexed_tx_graph.rs b/crates/chain/src/indexed_tx_graph.rs index 9f61788df7..4b468c6ffe 100644 --- a/crates/chain/src/indexed_tx_graph.rs +++ b/crates/chain/src/indexed_tx_graph.rs @@ -449,13 +449,11 @@ where /// that can be used to determine which transactions are canonical based on the provided /// parameters. The task handles the stateless canonicalization logic and can be polled /// for anchor verification requests. - /// - /// [`CanonicalView`]: crate::CanonicalView - pub fn canonical_task( + pub fn canonical_task( &'_ self, chain_tip: BlockId, params: CanonicalParams, - ) -> CanonicalTask<'_, A> { + ) -> CanonicalTask<'_, A, B> { self.graph.canonical_task(chain_tip, params) } } diff --git a/crates/chain/src/local_chain.rs b/crates/chain/src/local_chain.rs index 901f279172..13315846ee 100644 --- a/crates/chain/src/local_chain.rs +++ b/crates/chain/src/local_chain.rs @@ -4,8 +4,8 @@ use core::fmt; use core::ops::RangeBounds; use crate::collections::BTreeMap; -use crate::{Anchor, BlockId, CanonicalParams, CanonicalView, Merge, TxGraph}; -use bdk_core::{ChainQuery, ToBlockHash}; +use crate::{Anchor, BlockId, CanonicalParams, CanonicalTask, CanonicalView, Merge, TxGraph}; +use bdk_core::{ChainTask, TaskProgress, ToBlockHash, ToBlockTime}; pub use bdk_core::{CheckPoint, CheckPointEntry, CheckPointIter}; use bitcoin::block::Header; use bitcoin::BlockHash; @@ -86,28 +86,6 @@ impl PartialEq for LocalChain { // Methods for `LocalChain` impl LocalChain { - /// Check if a block is in the chain. - /// - /// # Arguments - /// * `block` - The block to check - /// * `chain_tip` - The chain tip to check against - /// - /// # Returns - /// * `Some(true)` if the block is in the chain - /// * `Some(false)` if the block is not in the chain - /// * `None` if it cannot be determined - pub fn is_block_in_chain(&self, block: BlockId, chain_tip: BlockId) -> Option { - let chain_tip_cp = match self.tip.get(chain_tip.height) { - // we can only determine whether `block` is in chain of `chain_tip` if `chain_tip` can - // be identified in chain - Some(cp) if cp.hash() == chain_tip.hash => cp, - _ => return None, - }; - chain_tip_cp - .get(block.height) - .map(|cp| cp.hash() == block.hash) - } - /// Get the chain tip. /// /// # Returns @@ -116,58 +94,6 @@ impl LocalChain { self.tip.block_id() } - /// Canonicalize a transaction graph using this chain. - /// - /// This method processes any type implementing [`ChainQuery`], handling all its requests - /// to determine which transactions are canonical, and returns the query's output. - /// - /// # Example - /// - /// ``` - /// # use bdk_chain::{CanonicalTask, CanonicalParams, TxGraph, local_chain::LocalChain}; - /// # use bdk_core::BlockId; - /// # use bitcoin::hashes::Hash; - /// # let tx_graph: TxGraph = TxGraph::default(); - /// # let chain = LocalChain::from_blocks([(0, bitcoin::BlockHash::all_zeros())].into_iter().collect()).unwrap(); - /// let chain_tip = chain.tip().block_id(); - /// let task = CanonicalTask::new(&tx_graph, chain_tip, CanonicalParams::default()); - /// let view = chain.canonicalize(task); - /// ``` - pub fn canonicalize(&self, mut task: Q) -> Q::Output - where - Q: ChainQuery, - { - let chain_tip = task.tip(); - while let Some(request) = task.next_query() { - let mut best_block_id = None; - for block_id in &request { - if self.is_block_in_chain(*block_id, chain_tip) == Some(true) { - best_block_id = Some(*block_id); - break; - } - } - task.resolve_query(best_block_id); - } - task.finish() - } - - /// Convenience method that runs both canonicalization phases and returns a [`CanonicalView`]. - /// - /// This is equivalent to: - /// ```ignore - /// let canonical_txs = chain.canonicalize(tx_graph.canonical_task(tip, params)); - /// let view = chain.canonicalize(canonical_txs.view_task(tx_graph)); - /// ``` - pub fn canonical_view( - &self, - tx_graph: &TxGraph, - tip: BlockId, - params: CanonicalParams, - ) -> CanonicalView { - let canonical_txs = self.canonicalize(tx_graph.canonical_task(tip, params)); - self.canonicalize(canonical_txs.view_task(tx_graph)) - } - /// Update the chain with a given [`Header`] at `height` which you claim is connected to a /// existing block in the chain. /// @@ -486,6 +412,96 @@ where Ok(changeset) } + /// Check if a block is in the chain. + /// + /// # Returns + /// * `Some(true)` if the block is in the chain + /// * `Some(false)` if the block is not in the chain + /// * `None` if it cannot be determined + pub fn is_block_in_chain(&self, block: BlockId, chain_tip: BlockId) -> Option { + let chain_tip_cp = match self.tip.get(chain_tip.height) { + Some(cp) if cp.hash() == chain_tip.hash => cp, + _ => return None, + }; + chain_tip_cp + .get(block.height) + .map(|cp| cp.hash() == block.hash) + } + + /// Drive a [`ChainTask`] to completion against this chain. + /// + /// This method processes any type implementing [`ChainTask`], handling all its requests + /// against this chain and returning the task's output. The chain responds with its data + /// type `D`, so `LocalChain

` responds with headers and `LocalChain` + /// responds with hashes. + pub fn run_task(&self, mut task: T) -> T::Output + where + T: ChainTask, + D: Clone, + { + loop { + match task.poll() { + TaskProgress::Advanced => continue, + TaskProgress::Done => return task.finish(), + TaskProgress::Query(heights) => { + debug_assert!(!heights.is_empty(), "TaskProgress::Query must not be empty"); + for height in heights { + let data = self + .get(height) + .filter(|cp| { + let chain_tip = task.tip(); + self.is_block_in_chain(cp.block_id(), chain_tip) == Some(true) + }) + .map(|cp| cp.data()); + task.resolve_query(height, data); + } + } + // This is a synchronous driver: every `Query` height is resolved before the next + // poll, so the task never has an in-flight query and cannot return this variant. + TaskProgress::Blocked => { + unreachable!( + "run_task resolves queries synchronously; nothing is ever in-flight" + ) + } + } + } + } + + /// Canonicalize a transaction graph against this chain and return a [`CanonicalView`]. + /// + /// This is equivalent to: + /// ```ignore + /// let (txs, queries) = chain.run_task(CanonicalTask::new(&tx_graph, tip, params)); + /// let view = chain.run_task(txs.view_task(tx_graph, queries)); + /// ``` + pub fn canonicalize( + &self, + tx_graph: &TxGraph, + tip: BlockId, + params: CanonicalParams, + ) -> CanonicalView { + let (txs, queries) = self.run_task(CanonicalTask::new(tx_graph, tip, params)); + self.run_task(txs.view_task(tx_graph, queries)) + } + + /// Like [`canonicalize`](Self::canonicalize), but also computes median-time-past (MTP) + /// values. Requires `D: ToBlockTime` (e.g. `LocalChain
`). + /// + /// See [`CanonicalViewTask::with_mtp`](crate::CanonicalViewTask::with_mtp) for what is + /// computed and the block data it requires. + pub fn canonicalize_with_mtp( + &self, + tx_graph: &TxGraph, + tip: BlockId, + params: CanonicalParams, + ) -> CanonicalView + where + D: ToBlockTime, + { + let (txs, queries) = self.run_task(CanonicalTask::new(tx_graph, tip, params)); + self.run_task(txs.view_task(tx_graph, queries).with_mtp()) + } + fn _check_changeset_is_applied(&self, changeset: &ChangeSet) -> bool { let mut cur = self.tip.clone(); for (&exp_height, exp_data) in changeset.blocks.iter().rev() { diff --git a/crates/chain/src/tx_graph.rs b/crates/chain/src/tx_graph.rs index 3c6ab69e59..8211226829 100644 --- a/crates/chain/src/tx_graph.rs +++ b/crates/chain/src/tx_graph.rs @@ -135,8 +135,8 @@ use crate::{Anchor, BlockId, Merge}; use alloc::collections::vec_deque::VecDeque; use alloc::sync::Arc; use alloc::vec::Vec; -use bdk_core::ConfirmationBlockTime; pub use bdk_core::TxUpdate; +use bdk_core::{ConfirmationBlockTime, ToBlockHash}; use bitcoin::{Amount, OutPoint, SignedAmount, Transaction, TxOut, Txid}; use core::fmt::{self, Formatter}; use core::ops::{Deref, RangeInclusive}; @@ -996,11 +996,11 @@ impl TxGraph { /// for anchor verification requests. /// /// [`CanonicalView`]: crate::CanonicalView - pub fn canonical_task( + pub fn canonical_task( &'_ self, chain_tip: BlockId, params: CanonicalParams, - ) -> CanonicalTask<'_, A> { + ) -> CanonicalTask<'_, A, B> { CanonicalTask::new(self, chain_tip, params) } } diff --git a/crates/chain/tests/test_canonical_view.rs b/crates/chain/tests/test_canonical_view.rs index 9b17d3d0b0..2672c56688 100644 --- a/crates/chain/tests/test_canonical_view.rs +++ b/crates/chain/tests/test_canonical_view.rs @@ -87,8 +87,7 @@ fn test_is_settled_boundary() { }; let _ = tx_graph.insert_anchor(txid, anchor_height_5); - let canonical_view = - chain.canonical_view(&tx_graph, chain.tip().block_id(), Default::default()); + let canonical_view = chain.canonicalize(&tx_graph, chain.tip().block_id(), Default::default()); let tip_height = canonical_view.tip().height; // Test min_confirmations = 1: Should be confirmed (has 6 confirmations) @@ -192,8 +191,7 @@ fn test_min_confirmations_with_untrusted_tx() { }; let _ = tx_graph.insert_anchor(txid, anchor); - let canonical_view = - chain.canonical_view(&tx_graph, chain.tip().block_id(), Default::default()); + let canonical_view = chain.canonicalize(&tx_graph, chain.tip().block_id(), Default::default()); let tip_height = canonical_view.tip().height; // Test with min_confirmations = 5 and everything tainted @@ -355,8 +353,7 @@ fn test_min_confirmations_multiple_transactions() { ); outpoints.push(outpoint2); - let canonical_view = - chain.canonical_view(&tx_graph, chain.tip().block_id(), Default::default()); + let canonical_view = chain.canonicalize(&tx_graph, chain.tip().block_id(), Default::default()); let tip_height = canonical_view.tip().height; // Test with min_confirmations = 5 @@ -493,7 +490,7 @@ fn test_balance_taint_propagates_through_unconfirmed_ancestry() { .into_iter() .collect::>(); - let view = chain.canonical_view(&tx_graph, chain.tip().block_id(), Default::default()); + let view = chain.canonicalize(&tx_graph, chain.tip().block_id(), Default::default()); // Our unspent owned outputs: `trusted` and `chained` (the others are spent). let utxos = [ @@ -546,7 +543,7 @@ fn test_balance_is_settled_is_authoritative_for_unconfirmed() { let _ = tx_graph.insert_tx(tx); let _ = tx_graph.insert_seen_at(txid, 1000); - let view = chain.canonical_view(&tx_graph, chain.tip().block_id(), Default::default()); + let view = chain.canonicalize(&tx_graph, chain.tip().block_id(), Default::default()); // An `is_settled` that claims everything is settled counts the (mature, non-coinbase) // unconfirmed output as settled rather than dropping it, even when it is tainted. @@ -618,7 +615,7 @@ fn test_balance_taint_stops_at_settled_ancestor() { .into_iter() .collect::>(); - let view = chain.canonical_view(&tx_graph, chain.tip().block_id(), Default::default()); + let view = chain.canonicalize(&tx_graph, chain.tip().block_id(), Default::default()); // `child` is the only UTXO (`settled_foreign:0` is spent by it). let by_op = view @@ -693,7 +690,7 @@ fn test_classify_immature_and_settled() { ); } - let view = chain.canonical_view(&tx_graph, chain.tip().block_id(), Default::default()); + let view = chain.canonicalize(&tx_graph, chain.tip().block_id(), Default::default()); let ops = [ OutPoint::new(coinbase_txid, 0), OutPoint::new(normal_txid, 0), @@ -787,7 +784,7 @@ fn test_balance_taint_shared_ancestor() { .into_iter() .collect::>(); - let view = chain.canonical_view(&tx_graph, chain.tip().block_id(), Default::default()); + let view = chain.canonicalize(&tx_graph, chain.tip().block_id(), Default::default()); // Both children descend from the same tainting `foreign`, so both are untrusted. let balance = view.balance( @@ -847,7 +844,7 @@ fn test_classify_skips_spent_and_unknown() { let _ = tx_graph.insert_seen_at(tx.compute_txid(), 1000); } - let view = chain.canonical_view(&tx_graph, chain.tip().block_id(), Default::default()); + let view = chain.canonicalize(&tx_graph, chain.tip().block_id(), Default::default()); let classified = view .classify_outpoints( @@ -895,7 +892,7 @@ fn test_immature_coinbase_stays_immature_when_unsettled() { }, ); - let view = chain.canonical_view(&tx_graph, chain.tip().block_id(), Default::default()); + let view = chain.canonicalize(&tx_graph, chain.tip().block_id(), Default::default()); let tip_height = view.tip().height; // Require 3 confirmations to be settled. @@ -942,7 +939,7 @@ fn test_mature_coinbase_is_settled_not_immature() { }, ); - let view = chain.canonical_view(&tx_graph, chain.tip().block_id(), Default::default()); + let view = chain.canonicalize(&tx_graph, chain.tip().block_id(), Default::default()); let tip_height = view.tip().height; let by_op = view @@ -999,7 +996,7 @@ fn test_unsettled_unknown_when_parent_root_missing() { } // `missing_root` is not inserted. - let view = chain.canonical_view(&tx_graph, chain.tip().block_id(), Default::default()); + let view = chain.canonicalize(&tx_graph, chain.tip().block_id(), Default::default()); let by_op = view .classify_outpoints( @@ -1054,7 +1051,7 @@ fn test_evicted_stale_anchored_tx_not_canonical() { let _ = tx_graph.insert_seen_at(txid, 100); let _ = tx_graph.insert_evicted_at(txid, 200); - let view = chain.canonical_view(&tx_graph, chain.tip().block_id(), Default::default()); + let view = chain.canonicalize(&tx_graph, chain.tip().block_id(), Default::default()); assert!( !view.txs().any(|tx| tx.txid == txid), "evicted leftover tx must not be canonical" @@ -1092,7 +1089,7 @@ fn test_classify_locked() { }, ); - let view = chain.canonical_view(&tx_graph, chain.tip().block_id(), Default::default()); + let view = chain.canonicalize(&tx_graph, chain.tip().block_id(), Default::default()); let ops = [OutPoint::new(txid, 0)]; // Timelock unmet @@ -1156,7 +1153,7 @@ fn test_classify_locked_descriptor_timelocks() { let _ = tx_graph.insert_seen_at(txid, 100); let classify = |chain: &LocalChain, tx_graph: &TxGraph| { - let view = chain.canonical_view(tx_graph, chain.tip().block_id(), Default::default()); + let view = chain.canonicalize(tx_graph, chain.tip().block_id(), Default::default()); let tip = view.tip().height; let ops = [OutPoint::new(txid, 0), OutPoint::new(txid, 1)]; view.classify_outpoints( diff --git a/crates/chain/tests/test_canonical_view_mtp.rs b/crates/chain/tests/test_canonical_view_mtp.rs new file mode 100644 index 0000000000..f29272caf9 --- /dev/null +++ b/crates/chain/tests/test_canonical_view_mtp.rs @@ -0,0 +1,181 @@ +#![cfg(feature = "miniscript")] + +//! Integration test for median-time-past (MTP) computation via +//! [`LocalChain::canonicalize_with_mtp`]. + +use std::collections::BTreeMap; + +use bdk_chain::{local_chain::LocalChain, ChainPosition, ConfirmationBlockTime, TxGraph}; +use bdk_testenv::{hash, utils::new_tx}; +use bitcoin::{ + block::Header, hashes::Hash, Amount, BlockHash, OutPoint, ScriptBuf, Transaction, TxIn, TxOut, +}; + +/// Build a block header with an explicit `time`. +fn header(prev_blockhash: BlockHash, time: u32) -> Header { + Header { + version: bitcoin::block::Version::default(), + prev_blockhash, + merkle_root: bitcoin::hash_types::TxMerkleNode::all_zeros(), + time, + bits: bitcoin::CompactTarget::default(), + nonce: 0, + } +} + +/// Build a connected chain of blocks `0..=tip_height` where each block's timestamp equals its +/// height (the block at height `h` has `time == h`). +/// +/// With timestamps equal to heights, the median-time-past at height `h` (the median of the 11 +/// timestamps in `h-10..=h`) is simply the middle height, `h - 5`. +fn chain_with_time_equal_to_height(tip_height: u32) -> LocalChain
{ + let mut headers = vec![header(BlockHash::all_zeros(), 0)]; // genesis at height 0 + for h in 1..=tip_height { + let prev = headers[(h - 1) as usize].block_hash(); + headers.push(header(prev, h)); + } + let blocks: BTreeMap = headers + .into_iter() + .enumerate() + .map(|(height, hdr)| (height as u32, hdr)) + .collect(); + 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, + chain: &LocalChain
, + 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. + let chain = chain_with_time_equal_to_height(12); + + // A parent with no anchor and no `seen_at`, and a child spending it confirmed at height 11. + // The parent is canonical only *transitively*, through the child's anchor. + let mut tx_graph = TxGraph::default(); + let parent = Transaction { + input: vec![TxIn { + previous_output: OutPoint::new(hash!("grandparent"), 0), + ..Default::default() + }], + output: vec![TxOut { + value: Amount::from_sat(60_000), + script_pubkey: ScriptBuf::new(), + }], + ..new_tx(0) + }; + let parent_txid = parent.compute_txid(); + let _ = tx_graph.insert_tx(parent); + let tx = Transaction { + input: vec![TxIn { + previous_output: OutPoint::new(parent_txid, 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(11).unwrap().block_id(), + confirmation_time: 0, + }, + ); + + // Canonicalize with MTP enabled. + let view = chain.canonicalize_with_mtp(&tx_graph, chain.tip().block_id(), Default::default()); + + // 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. Its output's `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. + let canonical_tx = view + .txs() + .find(|c| c.txid == txid) + .expect("tx is canonical"); + assert!(matches!(canonical_tx.pos, ChainPosition::Confirmed { .. })); + let txo = view.txout(OutPoint::new(txid, 0)).expect("output exists"); + assert_eq!(txo.prev_mtp, Some(5)); + + // The parent's own confirmation height is unknown, so its output has no `prev_mtp`, even + // though MTP is on and the chain holds the full window (the child's `Some(5)` shows both). + let parent_tx = view + .txs() + .find(|c| c.txid == parent_txid) + .expect("parent is canonical"); + assert!(matches!( + parent_tx.pos, + ChainPosition::Confirmed { + transitively: Some(_), + .. + } + )); + let parent_txo = view + .txout(OutPoint::new(parent_txid, 0)) + .expect("output exists"); + assert_eq!(parent_txo.prev_mtp, None); +} + +#[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. + 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_txo = view + .txout(OutPoint::new(genesis_txid, 0)) + .expect("output exists"); + assert_eq!(genesis_txo.prev_mtp, Some(0)); + + let low_txo = view + .txout(OutPoint::new(low_txid, 0)) + .expect("output exists"); + assert_eq!(low_txo.prev_mtp, Some(1)); +} diff --git a/crates/chain/tests/test_canonical_view_task.rs b/crates/chain/tests/test_canonical_view_task.rs index 397819f4fc..436e291681 100644 --- a/crates/chain/tests/test_canonical_view_task.rs +++ b/crates/chain/tests/test_canonical_view_task.rs @@ -2,9 +2,9 @@ mod common; -use bdk_chain::{CanonicalReason, ChainPosition}; +use bdk_chain::{local_chain::LocalChain, CanonicalReason, ChainPosition}; use bdk_testenv::{block_id, hash, local_chain}; -use bitcoin::Txid; +use bitcoin::{BlockHash, Txid}; use common::*; use std::collections::HashSet; @@ -13,7 +13,7 @@ fn test_assumed_canonical_scenarios() { // scenario: "txC spends txB; txB spends txA; txB is anchored; txC is assumed canonical" // create a local chain - let local_chain = local_chain![ + let local_chain: LocalChain = local_chain![ (0, hash!("genesis")), (1, hash!("block1")), (2, hash!("block2")), @@ -88,7 +88,7 @@ fn test_assumed_canonical_scenarios() { // build task & canonicalize let canonical_params = env.canonicalization_params; let canonical_task = env.tx_graph.canonical_task(chain_tip, canonical_params); - let canonical_txs = local_chain.canonicalize(canonical_task); + let (canonical_txs, queries) = local_chain.run_task(canonical_task); // assert canonical transactions let exp_canonical_txids: HashSet = exp_canonical_txs @@ -165,8 +165,8 @@ fn test_assumed_canonical_scenarios() { ]; // build task & resolve positions - let view_task = canonical_txs.view_task(&env.tx_graph); - let canonical_view = local_chain.canonicalize(view_task); + let view_task = canonical_txs.view_task(&env.tx_graph, queries); + let canonical_view = local_chain.run_task(view_task); // assert final positions for (tx_name, exp_position) in exp_positions { diff --git a/crates/chain/tests/test_indexed_tx_graph.rs b/crates/chain/tests/test_indexed_tx_graph.rs index c6e9443cec..2a46207fac 100644 --- a/crates/chain/tests/test_indexed_tx_graph.rs +++ b/crates/chain/tests/test_indexed_tx_graph.rs @@ -538,7 +538,7 @@ fn test_list_owned_txouts() { .unwrap_or_else(|| panic!("block must exist at {height}")); let canonical_view = - local_chain.canonical_view(graph.graph(), chain_tip, Default::default()); + local_chain.canonicalize(graph.graph(), chain_tip, Default::default()); let txouts = canonical_view .filter_outpoints(graph.index.outpoints().iter().cloned()) @@ -857,7 +857,7 @@ fn test_get_chain_position() { // check chain position let chain_pos = chain - .canonical_view(graph.graph(), chain.tip().block_id(), Default::default()) + .canonicalize(graph.graph(), chain.tip().block_id(), Default::default()) .txs() .find_map(|canon_tx| { if canon_tx.txid == txid { diff --git a/crates/chain/tests/test_tx_graph.rs b/crates/chain/tests/test_tx_graph.rs index 24b7b947de..af4780eb73 100644 --- a/crates/chain/tests/test_tx_graph.rs +++ b/crates/chain/tests/test_tx_graph.rs @@ -1015,7 +1015,7 @@ fn test_chain_spends() { let build_canonical_spends = |chain: &LocalChain, tx_graph: &TxGraph| -> HashMap { chain - .canonical_view(tx_graph, tip.block_id(), Default::default()) + .canonicalize(tx_graph, tip.block_id(), Default::default()) .filter_outpoints(tx_graph.all_txouts().map(|(op, _)| ((), op))) .filter_map(|(_, full_txo)| Some((full_txo.outpoint, full_txo.spent_by?))) .collect() @@ -1024,7 +1024,7 @@ fn test_chain_spends() { tx_graph: &TxGraph| -> HashMap> { chain - .canonical_view(tx_graph, tip.block_id(), Default::default()) + .canonicalize(tx_graph, tip.block_id(), Default::default()) .txs() .map(|canon_tx| (canon_tx.txid, canon_tx.pos)) .collect() @@ -1198,14 +1198,14 @@ fn transactions_inserted_into_tx_graph_are_not_canonical_until_they_have_an_anch .collect(); let chain = LocalChain::from_blocks(blocks).unwrap(); let canonical_txs: Vec<_> = chain - .canonical_view(&graph, chain.tip().block_id(), Default::default()) + .canonicalize(&graph, chain.tip().block_id(), Default::default()) .txs() .collect(); assert!(canonical_txs.is_empty()); // tx0 with seen_at should be returned by canonical txs let _ = graph.insert_seen_at(txids[0], 2); - let view = chain.canonical_view(&graph, chain.tip().block_id(), Default::default()); + let view = chain.canonicalize(&graph, chain.tip().block_id(), Default::default()); let mut canonical_txs = view.txs(); assert_eq!(canonical_txs.next().map(|tx| tx.txid).unwrap(), txids[0]); drop(canonical_txs); @@ -1213,7 +1213,7 @@ fn transactions_inserted_into_tx_graph_are_not_canonical_until_they_have_an_anch // tx1 with anchor is also canonical let _ = graph.insert_anchor(txids[1], block_id!(2, "B")); let canonical_txids: Vec<_> = chain - .canonical_view(&graph, chain.tip().block_id(), Default::default()) + .canonicalize(&graph, chain.tip().block_id(), Default::default()) .txs() .map(|tx| tx.txid) .collect(); diff --git a/crates/chain/tests/test_tx_graph_conflicts.rs b/crates/chain/tests/test_tx_graph_conflicts.rs index 2f6f1fd6bb..064a3aff94 100644 --- a/crates/chain/tests/test_tx_graph_conflicts.rs +++ b/crates/chain/tests/test_tx_graph_conflicts.rs @@ -1003,7 +1003,7 @@ fn test_tx_conflict_handling() { for scenario in scenarios { let env = init_graph(scenario.tx_templates.iter()); - let canonical_view = local_chain.canonical_view( + let canonical_view = local_chain.canonicalize( &env.tx_graph, chain_tip, env.canonicalization_params.clone(), @@ -1078,7 +1078,7 @@ fn test_tx_conflict_handling() { /// height is only an upper bound until its own anchor is known. #[test] fn transitively_confirmed_tx_reports_descendant_anchor_height() { - let chain = local_chain![ + let chain: LocalChain = local_chain![ (0, hash!("genesis")), (1, hash!("parent_block")), (5, hash!("child_block")) @@ -1110,7 +1110,7 @@ fn transitively_confirmed_tx_reports_descendant_anchor_height() { let parent_position = |tx_graph: &TxGraph| { chain - .canonical_view(tx_graph, tip, Default::default()) + .canonicalize(tx_graph, tip, Default::default()) .txs() .find(|tx| tx.txid == parent_txid) .expect("parent must be canonical") diff --git a/crates/core/src/block_queries.rs b/crates/core/src/block_queries.rs new file mode 100644 index 0000000000..d606c16cad --- /dev/null +++ b/crates/core/src/block_queries.rs @@ -0,0 +1,67 @@ +use crate::collections::BTreeMap; +use alloc::collections::BTreeSet; +use alloc::vec::Vec; + +/// Tracks block-height queries: which heights have been requested, +/// which are still pending, and the resolved data. +/// +/// This is a helper for [`ChainTask`](crate::ChainTask) implementors that need to +/// track which block heights have been requested from the chain source and which +/// responses have been received. It deduplicates requests and stores resolved blocks. +/// +/// # Usage +/// +/// 1. Call [`request`](Self::request) with the heights you need. It returns only the heights that +/// haven't been requested or resolved yet. +/// 2. Feed responses back via [`resolve`](Self::resolve). +/// 3. Look up resolved blocks with [`get`](Self::get). +/// 4. Check for outstanding requests with [`unresolved`](Self::unresolved). +pub struct BlockQueries { + pending: BTreeSet, + blocks: BTreeMap>, +} + +impl BlockQueries { + /// Creates an empty `BlockQueries`. + pub fn new() -> Self { + Self { + pending: BTreeSet::new(), + blocks: BTreeMap::new(), + } + } + + /// Filters out already-resolved/pending heights, inserts new ones into pending, + /// and returns the list of newly requested heights. + pub fn request(&mut self, heights: impl Iterator) -> Vec { + heights + .filter(|h| !self.blocks.contains_key(h)) + .filter(|h| self.pending.insert(*h)) + .collect() + } + + /// Marks a height as resolved. Removes from pending and inserts into blocks. + pub fn resolve(&mut self, height: u32, block: Option) { + let was_pending = self.pending.remove(&height); + self.blocks.insert(height, block); + debug_assert!( + was_pending, + "request must be previously unresolved to be resolved now" + ); + } + + /// Iterates over heights that are still pending (unresolved). + pub fn unresolved(&self) -> impl Iterator + '_ { + self.pending.iter().copied() + } + + /// Looks up a resolved block by height. + pub fn get(&self, height: u32) -> Option<&Option> { + self.blocks.get(&height) + } +} + +impl Default for BlockQueries { + fn default() -> Self { + Self::new() + } +} diff --git a/crates/core/src/chain_query.rs b/crates/core/src/chain_query.rs deleted file mode 100644 index 660c1c74ff..0000000000 --- a/crates/core/src/chain_query.rs +++ /dev/null @@ -1,41 +0,0 @@ -//! Trait for query-based canonicalization against blockchain data. -//! -//! The [`ChainQuery`] trait provides a sans-IO interface for algorithms that -//! need to verify block confirmations against a chain source. - -use crate::BlockId; -use alloc::vec::Vec; - -/// A request containing [`BlockId`]s to check for confirmation in the chain. -pub type ChainRequest = Vec; - -/// Response containing the best confirmed [`BlockId`], if any. -pub type ChainResponse = Option; - -/// A trait for types that verify block confirmations against blockchain data. -/// -/// This trait enables a sans-IO loop: the caller drives the task by repeatedly -/// calling [`next_query`](Self::next_query) and [`resolve_query`](Self::resolve_query). -/// Once `next_query` returns `None`, call [`finish`](Self::finish) to get the output. -/// -/// `resolve_query` must only be called after `next_query` returns `Some`. -/// Calling `resolve_query` or `finish` out of sequence is a programming error. -pub trait ChainQuery { - /// The final output type produced when the query process is complete. - type Output; - - /// Returns the chain tip used as the reference point for all queries. - fn tip(&self) -> BlockId; - - /// Returns the next query needed, or `None` if no more queries are required. - #[must_use] - fn next_query(&mut self) -> Option; - - /// Resolves a query with the given response. - fn resolve_query(&mut self, response: ChainResponse); - - /// Completes the query process and returns the final output. - /// - /// This should be called once [`next_query`](Self::next_query) returns `None`. - fn finish(self) -> Self::Output; -} diff --git a/crates/core/src/chain_task.rs b/crates/core/src/chain_task.rs new file mode 100644 index 0000000000..a65b1ef987 --- /dev/null +++ b/crates/core/src/chain_task.rs @@ -0,0 +1,132 @@ +//! Sans-IO trait for tasks that need to query block data from a chain source. +//! +//! See [`ChainTask`] for the cooperative protocol between a **task** (the implementor) +//! and a **driver** (the caller that has access to the chain source), including the +//! driver loop contract. + +use crate::BlockId; +use alloc::vec::Vec; +use bitcoin::BlockHash; + +/// Progress indicator returned by [`ChainTask::poll`]. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum TaskProgress { + /// Internal progress was made. The driver should call [`poll`](ChainTask::poll) again. + /// + /// Implementations should return this at stage transitions and after processing + /// individual items, rather than looping internally. This keeps `poll()` doing + /// one unit of work per call, giving the driver observability into progress and + /// an opportunity to cancel or log between steps. + Advanced, + /// The task needs block data at these **newly requested** heights. The driver should + /// resolve each height via [`resolve_query`](ChainTask::resolve_query) and call + /// [`poll`](ChainTask::poll) again. + /// + /// This variant is *additive*: a given height appears in exactly one `Query`, when it is + /// first needed. Implementations dedup against already-requested heights, so the vector is + /// guaranteed to be non-empty and to contain only heights the driver has not seen before. + /// A driver can therefore launch a fetch for each height without tracking duplicates. + Query(Vec), + /// The task cannot make progress until previously requested heights are resolved, and it + /// has no *new* heights to request. The driver should resolve at least one outstanding + /// height (see [`unresolved_queries`](ChainTask::unresolved_queries)) and call + /// [`poll`](ChainTask::poll) again. + /// + /// A synchronous driver that resolves every [`Query`](Self::Query) height before re-polling + /// never observes this variant (nothing is ever in-flight). It exists for streaming drivers + /// that issue fetches without blocking and poll opportunistically: `Query` says "launch I/O", + /// `Blocked` says "block until some lands". + Blocked, + /// The task is complete. The driver should call [`finish`](ChainTask::finish). + Done, +} + +/// A sans-IO task that queries block data by height from a chain source. +/// +/// `ChainTask` defines a cooperative protocol between a **task** (the implementor) +/// and a **driver** (the caller that has access to the chain source). The task +/// declares what block heights it needs, the driver fetches them, and the task +/// makes progress until it produces a final output. +/// +/// # The driver loop +/// +/// A correct driver calls [`poll`](Self::poll) in a loop, matching the returned +/// [`TaskProgress`]. A simple **synchronous** driver resolves each query inline: +/// +/// ```ignore +/// loop { +/// match task.poll() { +/// TaskProgress::Advanced => continue, +/// TaskProgress::Done => return task.finish(), +/// TaskProgress::Query(heights) => { +/// debug_assert!(!heights.is_empty()); +/// for h in heights { +/// let block = chain_source.get(h); +/// task.resolve_query(h, block); +/// } +/// } +/// // A synchronous driver resolves every query before re-polling, so nothing is +/// // ever in-flight and this variant cannot occur. +/// TaskProgress::Blocked => unreachable!(), +/// } +/// } +/// ``` +/// +/// A **streaming** driver instead launches fetches on [`Query`](TaskProgress::Query) without +/// blocking and re-polls; when the task returns [`Blocked`](TaskProgress::Blocked) +/// it blocks on the next fetch to complete, resolves it, and re-polls. +/// +/// # The block type `B` +/// +/// The generic parameter `B` is the block data the task receives from the driver. +/// It defaults to [`BlockHash`] but can be any type (e.g. [`Header`](bitcoin::block::Header)) +/// depending on the chain source. A response of `None` means the chain source has +/// no block at that height. +/// +/// # Contract for implementors +/// +/// - [`poll`](Self::poll) should do one logical unit of work and return: +/// - [`TaskProgress::Advanced`] when internal progress was made (e.g. a stage transition or +/// processing a single item). The driver will call `poll()` again. +/// - [`TaskProgress::Query`] when the task needs block data at one or more *newly requested* +/// heights. Dedup against already-requested heights so each height is announced once and the +/// vector is never empty. +/// - [`TaskProgress::Blocked`] when the task has no new heights to request but cannot progress +/// until previously requested heights are resolved. +/// - [`TaskProgress::Done`] when all processing is complete. +/// - Prefer returning [`TaskProgress::Advanced`] over looping internally. This gives the driver +/// control between steps for progress reporting, cancellation, or logging. +/// - [`finish`](Self::finish) consumes the task and produces the final output. It should only be +/// called after `poll` returns [`TaskProgress::Done`]. +pub trait ChainTask { + /// The final output produced when the task completes. + type Output; + + /// The chain tip that serves as the reference point for all queries. + fn tip(&self) -> BlockId; + + /// Drive the task forward, returning its progress status. + /// + /// Each call should perform one logical unit of work. The driver calls this + /// in a loop, matching the returned [`TaskProgress`]. + fn poll(&mut self) -> TaskProgress; + + /// Provides the block data (or `None`) for a previously requested `height`. + /// + /// A response of `None` signals that the chain source has no block at that + /// height. The task records this so it can distinguish "not yet queried" + /// from "queried but absent." + fn resolve_query(&mut self, height: u32, response: Option); + + /// Returns heights that have been requested but not yet resolved. + /// + /// This is useful for retrying failed queries or reporting progress. + fn unresolved_queries<'a>(&'a self) -> impl Iterator + 'a; + + /// Consumes the task and returns the final output. + /// + /// # Panics + /// + /// May panic if called before [`poll`](Self::poll) returns [`TaskProgress::Done`]. + fn finish(self) -> Self::Output; +} diff --git a/crates/core/src/lib.rs b/crates/core/src/lib.rs index c9c97f3ef8..12a435cf89 100644 --- a/crates/core/src/lib.rs +++ b/crates/core/src/lib.rs @@ -89,5 +89,8 @@ pub use merge::*; pub mod spk_client; -mod chain_query; -pub use chain_query::*; +mod chain_task; +pub use chain_task::*; + +mod block_queries; +pub use block_queries::*; diff --git a/crates/electrum/tests/test_electrum.rs b/crates/electrum/tests/test_electrum.rs index 514d4015d0..d25587e4ab 100644 --- a/crates/electrum/tests/test_electrum.rs +++ b/crates/electrum/tests/test_electrum.rs @@ -57,7 +57,7 @@ fn get_balance( ) -> anyhow::Result { let outpoints = recv_graph.index.outpoints().clone(); let balance = recv_chain - .canonical_view( + .canonicalize( recv_graph.graph(), recv_chain.tip().block_id(), Default::default(), @@ -181,7 +181,7 @@ pub fn detect_receive_tx_cancel() -> anyhow::Result<()> { .chain_tip(chain.tip()) .spks_with_indexes(graph.index.all_spks().clone()) .expected_spk_txids( - { chain.canonical_view(graph.graph(), chain.tip().block_id(), Default::default()) } + { chain.canonicalize(graph.graph(), chain.tip().block_id(), Default::default()) } .list_expected_spk_txids(&graph.index, ..), ); let sync_response = client.sync(sync_request, BATCH_SIZE, true)?; @@ -210,7 +210,7 @@ pub fn detect_receive_tx_cancel() -> anyhow::Result<()> { .chain_tip(chain.tip()) .spks_with_indexes(graph.index.all_spks().clone()) .expected_spk_txids( - { chain.canonical_view(graph.graph(), chain.tip().block_id(), Default::default()) } + { chain.canonicalize(graph.graph(), chain.tip().block_id(), Default::default()) } .list_expected_spk_txids(&graph.index, ..), ); let sync_response = client.sync(sync_request, BATCH_SIZE, true)?; diff --git a/crates/esplora/tests/async_ext.rs b/crates/esplora/tests/async_ext.rs index a14a8397c9..7260842237 100644 --- a/crates/esplora/tests/async_ext.rs +++ b/crates/esplora/tests/async_ext.rs @@ -100,7 +100,7 @@ pub async fn detect_receive_tx_cancel() -> anyhow::Result<()> { .chain_tip(chain.tip()) .spks_with_indexes(graph.index.all_spks().clone()) .expected_spk_txids( - { chain.canonical_view(graph.graph(), chain.tip().block_id(), Default::default()) } + { chain.canonicalize(graph.graph(), chain.tip().block_id(), Default::default()) } .list_expected_spk_txids(&graph.index, ..), ); let sync_response = client.sync(sync_request, 1).await?; @@ -129,7 +129,7 @@ pub async fn detect_receive_tx_cancel() -> anyhow::Result<()> { .chain_tip(chain.tip()) .spks_with_indexes(graph.index.all_spks().clone()) .expected_spk_txids( - { chain.canonical_view(graph.graph(), chain.tip().block_id(), Default::default()) } + { chain.canonicalize(graph.graph(), chain.tip().block_id(), Default::default()) } .list_expected_spk_txids(&graph.index, ..), ); let sync_response = client.sync(sync_request, 1).await?; diff --git a/crates/esplora/tests/blocking_ext.rs b/crates/esplora/tests/blocking_ext.rs index 1095f15255..11924e9933 100644 --- a/crates/esplora/tests/blocking_ext.rs +++ b/crates/esplora/tests/blocking_ext.rs @@ -97,7 +97,7 @@ pub fn detect_receive_tx_cancel() -> anyhow::Result<()> { .chain_tip(chain.tip()) .spks_with_indexes(graph.index.all_spks().clone()) .expected_spk_txids( - { chain.canonical_view(graph.graph(), chain.tip().block_id(), Default::default()) } + { chain.canonicalize(graph.graph(), chain.tip().block_id(), Default::default()) } .list_expected_spk_txids(&graph.index, ..), ); let sync_response = client.sync(sync_request, 1)?; @@ -126,7 +126,7 @@ pub fn detect_receive_tx_cancel() -> anyhow::Result<()> { .chain_tip(chain.tip()) .spks_with_indexes(graph.index.all_spks().clone()) .expected_spk_txids( - { chain.canonical_view(graph.graph(), chain.tip().block_id(), Default::default()) } + { chain.canonicalize(graph.graph(), chain.tip().block_id(), Default::default()) } .list_expected_spk_txids(&graph.index, ..), ); let sync_response = client.sync(sync_request, 1)?; From acca4fb98749a2441f7a4cf9ecfcbf136d7c447e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Wed, 17 Jun 2026 07:36:45 +0000 Subject: [PATCH 2/9] refactor(core,chain): extract `BlockQueries::resolve_candidates` Both canonicalization phases checked a tx's anchors against fetched blocks with identical logic: scan for a resolved, matching block; else request the missing heights; else wait on in-flight queries; else give up. The two copies had already drifted (the phase-2 copy still carried the old `has_unresolved_heights` shape with a latent empty-`Query` edge). Extract that logic as `BlockQueries::resolve_candidates`, returning a `BlockCandidateResolution` enum (`Confirmed` / `Query` / `Awaiting` / `NotConfirmed`). It lives on `BlockQueries` since it is an operation on that type, and is decoupled from `Anchor`: callers pass the candidates as `(payload, BlockId)` pairs, so it needs only a `BlockId` per candidate rather than the `Anchor` trait. Each call site now maps the four outcomes onto its own queue handling and `TaskProgress`. Adds unit tests covering all four outcomes, including the `Awaiting` branch that the synchronous driver never hits. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/chain/src/canonical_task.rs | 82 +++++------ crates/chain/src/canonical_view_task.rs | 55 +++----- crates/core/src/block_queries.rs | 176 ++++++++++++++++++++++++ 3 files changed, 226 insertions(+), 87 deletions(-) diff --git a/crates/chain/src/canonical_task.rs b/crates/chain/src/canonical_task.rs index a8e239045b..93d190e7dd 100644 --- a/crates/chain/src/canonical_task.rs +++ b/crates/chain/src/canonical_task.rs @@ -5,7 +5,9 @@ use alloc::boxed::Box; use alloc::collections::BTreeSet; use alloc::sync::Arc; use alloc::vec::Vec; -use bdk_core::{BlockId, BlockQueries, ChainTask, TaskProgress, ToBlockHash}; +use bdk_core::{ + BlockCandidateResolution, BlockId, BlockQueries, ChainTask, TaskProgress, ToBlockHash, +}; use bitcoin::{Transaction, Txid}; type CanonicalMap = HashMap>>; @@ -117,59 +119,37 @@ impl<'g, A: Anchor, B: ToBlockHash> ChainTask for CanonicalTask<'g, A, B> { return TaskProgress::Advanced; } - let mut best_anchor = Option::::None; - for a in anchors.iter() { - let h = a.anchor_block().height; - // A block that is either unresolved (`None`), absent at this height - // (`Some(None)`), or present but mismatched does not confirm this anchor. - if let Some(Some(b)) = self.queries.get(h) { - if b.to_blockhash() == a.anchor_block().hash { - best_anchor = Some(a.clone()); - break; - } - } - } - - if let Some(a) = best_anchor { - self.mark_canonical(txid, tx, CanonicalReason::from_anchor(a)); - return TaskProgress::Advanced; - } - - // No resolved anchor matched. `request` returns only heights that are neither - // resolved nor already in-flight, so a non-empty result is genuinely new work - // to fetch. This keeps `Query` additive and never empty. - let heights = self + match self .queries - .request(anchors.iter().map(|a| a.anchor_block().height)); - if !heights.is_empty() { - self.unprocessed_anchored_txs - .push_front((txid, tx, anchors)); - return TaskProgress::Query(heights); - } - - // No new heights to request. If any anchor height is still in-flight (requested - // but unresolved), we can't decide this tx yet — wait for the driver. Otherwise - // every anchor height is resolved and none matched, so the tx is not confirmed. - if anchors - .iter() - .any(|a| self.queries.get(a.anchor_block().height).is_none()) + .resolve_candidates(anchors.iter().map(|a| (a, a.anchor_block()))) { - self.unprocessed_anchored_txs - .push_front((txid, tx, anchors)); - return TaskProgress::Blocked; + BlockCandidateResolution::Confirmed(a) => { + self.mark_canonical(txid, tx, CanonicalReason::from_anchor(a.clone())); + } + BlockCandidateResolution::Query(heights) => { + self.unprocessed_anchored_txs + .push_front((txid, tx, anchors)); + return TaskProgress::Query(heights); + } + BlockCandidateResolution::Awaiting => { + self.unprocessed_anchored_txs + .push_front((txid, tx, anchors)); + return TaskProgress::Blocked; + } + BlockCandidateResolution::NotConfirmed => { + // No anchor confirms this tx; defer to the leftover stage. + self.unprocessed_leftover_txs.push_back(( + txid, + tx, + anchors + .iter() + .last() + .expect("must have at least one anchor") + .anchor_block() + .height, + )); + } } - - // No confirmed anchor found. - self.unprocessed_leftover_txs.push_back(( - txid, - tx, - anchors - .iter() - .last() - .expect("must have at least one anchor") - .anchor_block() - .height, - )); } else { self.current_stage.next_stage(); } diff --git a/crates/chain/src/canonical_view_task.rs b/crates/chain/src/canonical_view_task.rs index b215c3f894..841c921c16 100644 --- a/crates/chain/src/canonical_view_task.rs +++ b/crates/chain/src/canonical_view_task.rs @@ -6,7 +6,10 @@ use crate::tx_graph::TxDescendants; use alloc::collections::BTreeSet; use alloc::vec::Vec; -use bdk_core::{BlockId, BlockQueries, ChainTask, TaskProgress, ToBlockHash, ToBlockTime}; +use bdk_core::{ + BlockCandidateResolution, BlockId, BlockQueries, ChainTask, TaskProgress, ToBlockHash, + ToBlockTime, +}; use bitcoin::{OutPoint, Txid}; use crate::{canonical::CanonicalEntry, Anchor, CanonicalView, ChainPosition, TxGraph}; @@ -144,44 +147,24 @@ impl<'g, A: Anchor, B: ToBlockHash> ChainTask for CanonicalViewTask<'g, A, B> match self.current_stage { ViewStage::ResolvingPositions => { if let Some((txid, anchors)) = self.unprocessed_anchor_checks.pop_front() { - let mut best_anchor = Option::::None; - for a in anchors.iter() { - let h = a.anchor_block().height; - // Only a resolved, present, matching block confirms this anchor. - if let Some(Some(b)) = self.queries.get(h) { - if b.to_blockhash() == a.anchor_block().hash { - best_anchor = Some(a.clone()); - break; - } - } - } - - if let Some(anchor) = best_anchor { - self.direct_anchors.insert(txid, anchor); - return TaskProgress::Advanced; - } - - // No resolved anchor matched. `request` is additive, so a non-empty result is - // genuinely new work to fetch and is never empty. - let heights = self + match self .queries - .request(anchors.iter().map(|a| a.anchor_block().height)); - if !heights.is_empty() { - self.unprocessed_anchor_checks.push_front((txid, anchors)); - return TaskProgress::Query(heights); - } - - // No new heights. If any anchor height is still in-flight, wait for the driver; - // otherwise all are resolved and none matched (no direct anchor for this tx). - if anchors - .iter() - .any(|a| self.queries.get(a.anchor_block().height).is_none()) + .resolve_candidates(anchors.iter().map(|a| (a, a.anchor_block()))) { - self.unprocessed_anchor_checks.push_front((txid, anchors)); - return TaskProgress::Blocked; + BlockCandidateResolution::Confirmed(anchor) => { + self.direct_anchors.insert(txid, anchor.clone()); + } + BlockCandidateResolution::Query(heights) => { + self.unprocessed_anchor_checks.push_front((txid, anchors)); + return TaskProgress::Query(heights); + } + BlockCandidateResolution::Awaiting => { + self.unprocessed_anchor_checks.push_front((txid, anchors)); + return TaskProgress::Blocked; + } + // No anchor confirms this tx; leave it without a direct anchor. + BlockCandidateResolution::NotConfirmed => {} } - - // No confirmed anchor found for this tx TaskProgress::Advanced } else { self.current_stage = ViewStage::FetchingMtpBlocks; diff --git a/crates/core/src/block_queries.rs b/crates/core/src/block_queries.rs index d606c16cad..334161f587 100644 --- a/crates/core/src/block_queries.rs +++ b/crates/core/src/block_queries.rs @@ -1,4 +1,5 @@ use crate::collections::BTreeMap; +use crate::{BlockId, ToBlockHash}; use alloc::collections::BTreeSet; use alloc::vec::Vec; @@ -60,8 +61,183 @@ impl BlockQueries { } } +impl BlockQueries { + /// Resolves a set of candidate blocks against the blocks fetched so far. + /// + /// Each candidate is a pair of an opaque payload and a [`BlockId`]. A candidate is *in-chain* + /// when the block resolved at that `BlockId`'s height is present and its hash matches; the + /// first such candidate's payload is returned as + /// [`Confirmed`](BlockCandidateResolution::Confirmed). + /// + /// When no candidate is confirmed, the result tells the caller how to make progress: + /// - [`Query`](BlockCandidateResolution::Query): heights that still need fetching. These come + /// from [`request`](Self::request), so the list is *additive* (each height announced once) + /// and never empty. + /// - [`Awaiting`](BlockCandidateResolution::Awaiting): nothing new to request, but some heights + /// are still in-flight; wait for them to resolve. + /// - [`NotConfirmed`](BlockCandidateResolution::NotConfirmed): every candidate height is + /// resolved and none matched. + /// + /// `candidates` is cloned and traversed up to three times, so prefer a cheaply cloneable + /// iterator (e.g. a `map` over a slice or set). + pub fn resolve_candidates( + &mut self, + candidates: impl Iterator + Clone, + ) -> BlockCandidateResolution { + // A candidate is confirmed only by a resolved, present block whose hash matches. Unresolved + // (`None`), absent (`Some(None)`), or mismatched blocks do not confirm it. + for (c, id) in candidates.clone() { + if let Some(Some(b)) = self.get(id.height) { + if b.to_blockhash() == id.hash { + return BlockCandidateResolution::Confirmed(c.clone()); + } + } + } + + let heights = self.request(candidates.clone().map(|(_, id)| id.height)); + if !heights.is_empty() { + return BlockCandidateResolution::Query(heights); + } + + // Nothing new to request: wait if any candidate height is still in-flight, otherwise all + // are resolved and none matched. + if candidates + .clone() + .any(|(_, id)| self.get(id.height).is_none()) + { + return BlockCandidateResolution::Awaiting; + } + + BlockCandidateResolution::NotConfirmed + } +} + impl Default for BlockQueries { fn default() -> Self { Self::new() } } + +/// Outcome of [`BlockQueries::resolve_candidates`]. +/// +/// A "candidate" is some value `C` that claims to live at a particular [`BlockId`] (height + +/// hash). Resolution checks those claims against the blocks fetched so far and reports which of +/// four states applies. The generic `C` is carried through opaquely so the confirmed candidate can +/// be returned to the caller. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum BlockCandidateResolution { + /// One candidate's block is resolved and its hash matches, so this candidate is in-chain. + Confirmed(C), + /// These *newly requested* heights must be fetched before the candidates can be decided. + /// Always non-empty. + Query(Vec), + /// Nothing new to request, but some candidate heights are still in-flight; the candidates + /// can't be decided until the driver resolves them. + Awaiting, + /// Every candidate height is resolved and none match, so no candidate is in-chain. + NotConfirmed, +} + +#[cfg(test)] +mod test { + use super::*; + use alloc::vec; + use bitcoin::{hashes::Hash, BlockHash}; + + fn bhash(n: u8) -> BlockHash { + BlockHash::from_byte_array([n; 32]) + } + + fn id(height: u32, hash: u8) -> BlockId { + BlockId { + height, + hash: bhash(hash), + } + } + + #[test] + fn resolve_candidates_scenarios() { + struct Case { + name: &'static str, + /// Pre-populate the `BlockQueries` to set up the scenario. + setup: fn(&mut BlockQueries), + candidates: Vec, + expected: BlockCandidateResolution, + /// Heights still outstanding after the call. + expected_unresolved: Vec, + } + + let cases = [ + Case { + name: "confirmed when the block is resolved and its hash matches", + setup: |q| { + q.request([5].into_iter()); + q.resolve(5, Some(bhash(5))); + }, + candidates: vec![id(5, 5)], + expected: BlockCandidateResolution::Confirmed(id(5, 5)), + expected_unresolved: vec![], + }, + Case { + name: "confirmed picks the matching candidate among several", + setup: |q| { + q.request([4, 5].into_iter()); + q.resolve(4, Some(bhash(99))); // resolved but mismatched + q.resolve(5, Some(bhash(5))); // matches + }, + candidates: vec![id(4, 4), id(5, 5)], + expected: BlockCandidateResolution::Confirmed(id(5, 5)), + expected_unresolved: vec![], + }, + Case { + name: "query for a height not yet requested", + setup: |_q| {}, + candidates: vec![id(7, 7)], + expected: BlockCandidateResolution::Query(vec![7]), + expected_unresolved: vec![7], // the call records the request + }, + Case { + name: "awaiting when a requested height is still in-flight", + setup: |q| { + q.request([7].into_iter()); + }, + candidates: vec![id(7, 7)], + expected: BlockCandidateResolution::Awaiting, + expected_unresolved: vec![7], + }, + Case { + name: "not confirmed when the block is resolved but mismatched", + setup: |q| { + q.request([7].into_iter()); + q.resolve(7, Some(bhash(99))); + }, + candidates: vec![id(7, 7)], + expected: BlockCandidateResolution::NotConfirmed, + expected_unresolved: vec![], + }, + Case { + name: "not confirmed when the block is absent", + setup: |q| { + q.request([7].into_iter()); + q.resolve(7, None); + }, + candidates: vec![id(7, 7)], + expected: BlockCandidateResolution::NotConfirmed, + expected_unresolved: vec![], + }, + ]; + + for case in cases { + let mut q = BlockQueries::::new(); + (case.setup)(&mut q); + + let res = q.resolve_candidates(case.candidates.iter().map(|id| (*id, *id))); + assert_eq!(res, case.expected, "scenario: {}", case.name); + assert!( + q.unresolved().eq(case.expected_unresolved.iter().copied()), + "scenario: {} (unresolved mismatch)", + case.name + ); + } + } +} From 7cdb0c8421b5a96f575a37c361ee2c7b2c20bce9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Thu, 18 Jun 2026 21:53:46 +0000 Subject: [PATCH 3/9] refactor(core,chain): share `median_time_past` across MTP call sites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both `CheckPoint::median_time_past` and the canonical view task's MTP computation implemented the same BIP-113 median (11-block window, higher-middle value). Extract a `bdk_core::median_time_past(height, block_time)` helper — abstracting the timestamp source via a closure — so the rule lives in one place; both sites now delegate to it. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/chain/src/canonical_view_task.rs | 25 ++++++--------- crates/core/src/checkpoint.rs | 41 ++++++++++++++----------- 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/crates/chain/src/canonical_view_task.rs b/crates/chain/src/canonical_view_task.rs index 841c921c16..8df8f7c7a6 100644 --- a/crates/chain/src/canonical_view_task.rs +++ b/crates/chain/src/canonical_view_task.rs @@ -7,8 +7,8 @@ use alloc::collections::BTreeSet; use alloc::vec::Vec; use bdk_core::{ - BlockCandidateResolution, BlockId, BlockQueries, ChainTask, TaskProgress, ToBlockHash, - ToBlockTime, + median_time_past, BlockCandidateResolution, BlockId, BlockQueries, ChainTask, TaskProgress, + ToBlockHash, ToBlockTime, }; use bitcoin::{OutPoint, Txid}; @@ -222,21 +222,16 @@ impl<'g, A: Anchor, B: ToBlockHash> ChainTask for CanonicalViewTask<'g, A, B> } fn finish(self) -> Self::Output { - // Helper: compute MTP for a given height from the blocks map. Returns `None` when MTP - // was not enabled, so callers do not each repeat that check. + // Helper: compute MTP for a given height from the blocks fetched into `queries`. + // Returns `None` when MTP was not enabled, so callers do not each repeat that check. let try_compute_mtp_at = |h: u32| -> Option { let extract_time = self.extract_time?; - let start = h.saturating_sub(10); - let mut ts: Vec = (start..=h) - .map(|mtp_h| { - self.queries - .get(mtp_h) - .and_then(|b| b.as_ref()) - .map(extract_time) - }) - .collect::>>()?; - ts.sort_unstable(); - Some(ts[ts.len() / 2]) + median_time_past(h, |mtp_h| { + self.queries + .get(mtp_h) + .and_then(|b| b.as_ref()) + .map(extract_time) + }) }; // Compute tip MTP. diff --git a/crates/core/src/checkpoint.rs b/crates/core/src/checkpoint.rs index 7be0b40fe5..971a2c45e8 100644 --- a/crates/core/src/checkpoint.rs +++ b/crates/core/src/checkpoint.rs @@ -197,6 +197,26 @@ impl ToBlockTime for Header { } } +/// Number of blocks used to compute the median time past (BIP-0113). +const MTP_BLOCK_COUNT: u32 = 11; + +/// Compute the median time past (MTP) at `height` from block timestamps (BIP-0113). +/// +/// MTP is the median of the timestamps of the 11 blocks at heights `height-10..=height`. +/// `block_time` returns the timestamp of the block at a given height, or `None` if it is +/// unavailable; if any of the 11 blocks is missing this returns `None`. +/// +/// Note: this is a pseudo-median: for an even count it takes the higher of the two middle values +/// rather than averaging them, per the BIP-0113 specification. +pub fn median_time_past(height: u32, block_time: impl Fn(u32) -> Option) -> Option { + let start = height.saturating_sub(MTP_BLOCK_COUNT - 1); + let mut timestamps = (start..=height) + .map(block_time) + .collect::>>()?; + timestamps.sort_unstable(); + Some(timestamps[timestamps.len() / 2]) +} + impl PartialEq for CheckPoint { fn eq(&self, other: &Self) -> bool { let self_cps = self.iter().map(|cp| cp.block_id()); @@ -363,8 +383,6 @@ impl CheckPoint where D: ToBlockHash + fmt::Debug + Clone, { - const MTP_BLOCK_COUNT: u32 = 11; - /// Construct a new base [`CheckPoint`] from given `height` and `data` at the front of a linked /// list. pub fn new(height: u32, data: D) -> Self { @@ -394,22 +412,9 @@ where where D: ToBlockTime, { - let current_height = self.height(); - let earliest_height = current_height.saturating_sub(Self::MTP_BLOCK_COUNT - 1); - - let mut timestamps = (earliest_height..=current_height) - .map(|height| { - // Return `None` for missing blocks or missing block times - let cp = self.get(height)?; - let block_time = cp.data_ref().to_blocktime(); - Some(block_time) - }) - .collect::>>()?; - timestamps.sort_unstable(); - - // If there are more than 1 middle values, use the higher middle value. - // This is mathematically incorrect, but this is the BIP-0113 specification. - Some(timestamps[timestamps.len() / 2]) + median_time_past(self.height(), |height| { + Some(self.get(height)?.data_ref().to_blocktime()) + }) } /// Construct from an iterator of block data. From 6c3ba124e7d41cac7468f43933c6a24d3ff10689 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Thu, 10 Sep 2026 10:57:35 +0000 Subject: [PATCH 4/9] docs(chain): document that MTP needs a contiguous 11-block window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An MTP at height `h` medians the timestamps at `h-10..=h`, so any height in that window the driver cannot resolve makes the whole result `None` — silently, since a gap is indistinguishable from a reorged-out block. This is the normal case for a `LocalChain` built by syncing: electrum and esplora insert a handful of checkpoints, not every height, so such a chain yields `None` for both `prev_mtp` and `tip_mtp` until those clients fetch the preceding window for each confirmed height. Document the requirement on `with_mtp` and pin the behaviour with a test over a chain holding only heights {0, 50, 100}, which also shows that the gaps do not affect canonicalization itself. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NND8BhEmYTYdLRv4dbUUro --- crates/chain/src/canonical_view_task.rs | 18 ++++++++++ crates/chain/tests/test_canonical_view_mtp.rs | 34 +++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/crates/chain/src/canonical_view_task.rs b/crates/chain/src/canonical_view_task.rs index 8df8f7c7a6..774efef6b0 100644 --- a/crates/chain/src/canonical_view_task.rs +++ b/crates/chain/src/canonical_view_task.rs @@ -126,6 +126,24 @@ impl<'g, A: Anchor, B: ToBlockHash + ToBlockTime> CanonicalViewTask<'g, A, B> { /// [`CanonicalTxOut::prev_mtp`](crate::CanonicalTxOut::prev_mtp). For the tip it computes /// MTP(tip.height) (per BIP-113), accessible via /// [`tip_mtp()`](crate::Canonical::tip_mtp). + /// + /// # `None` means unknown, not zero + /// + /// An MTP at height `h` is the median of the timestamps at `h-10..=h`, so it is `Some` + /// only when the driver resolves all eleven of those heights. A window with any gap in it + /// yields `None`, since a partial window has no defined median, and a gap cannot be told + /// apart from a block that is genuinely absent. + /// + /// This is not an error, and a driver is under no obligation to hold every height. A + /// [`LocalChain`](crate::local_chain::LocalChain) built by syncing (electrum, esplora) + /// keeps a handful of checkpoints rather than every block, so it yields `None` for both + /// `prev_mtp` and `tip_mtp` until the sync client fetches the preceding window for each + /// confirmed height. + /// + /// Treat `None` as "MTP unknown here". It is safe for display and for reporting, but a + /// caller evaluating a BIP-68 or BIP-113 timelock cannot decide the lock from it, and + /// must not read it as "no timelock" or as a timestamp of zero, either of which would + /// report an immature coin as spendable. pub fn with_mtp(mut self) -> Self { self.extract_time = Some(B::to_blocktime); self diff --git a/crates/chain/tests/test_canonical_view_mtp.rs b/crates/chain/tests/test_canonical_view_mtp.rs index f29272caf9..0b53dc07cb 100644 --- a/crates/chain/tests/test_canonical_view_mtp.rs +++ b/crates/chain/tests/test_canonical_view_mtp.rs @@ -179,3 +179,37 @@ fn prev_mtp_saturates_for_low_confirmation_heights() { .expect("output exists"); assert_eq!(low_txo.prev_mtp, Some(1)); } + +#[test] +fn sparse_chain_yields_no_mtp_but_still_canonicalizes() { + // A chain holding only heights {0, 50, 100} — the shape a synced `LocalChain` has, since + // electrum/esplora insert a handful of checkpoints rather than every height. + let blocks: BTreeMap = { + let genesis = header(BlockHash::all_zeros(), 0); + // The heights are not adjacent, so `prev_blockhash` linkage between them is arbitrary; + // `LocalChain` only requires that it connects from genesis. + let at_50 = header(genesis.block_hash(), 50); + let at_100 = header(at_50.block_hash(), 100); + [(0, genesis), (50, at_50), (100, at_100)].into() + }; + let chain = LocalChain::from_blocks(blocks).expect("chain connects from genesis"); + let mut tx_graph = TxGraph::default(); + + let txid = confirm_tx_at(&mut tx_graph, &chain, 1, 50); + + let view = chain.canonicalize_with_mtp(&tx_graph, chain.tip().block_id(), Default::default()); + + // Canonicalization itself is unaffected by the gaps. + let canonical_tx = view + .txs() + .find(|c| c.txid == txid) + .expect("tx is canonical"); + assert!(matches!(canonical_tx.pos, ChainPosition::Confirmed { .. })); + + // But MTP needs *every* height in an 11-block window: `prev_mtp` would need 39..=49 and + // `tip_mtp` would need 90..=100, none of which this chain holds. Both are silently `None` + // rather than an error — a gap is indistinguishable from a reorged-out block. + let txo = view.txout(OutPoint::new(txid, 0)).expect("output exists"); + assert_eq!(txo.prev_mtp, None); + assert_eq!(view.tip_mtp(), None); +} From c40e419a049cf73392cc029e613d61b77245ad28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Thu, 10 Sep 2026 10:58:30 +0000 Subject: [PATCH 5/9] docs(core,chain): tighten the `ChainTask` driver contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gaps in the protocol between a task and its driver, one on each side, plus one at the hand-off between the two canonicalization phases. Task side: `resolve_query(height, None)` means "the chain source has no block at this height", and it is terminal — the height leaves the pending set and is never requested again. A driver that passed `None` on an I/O error would permanently record a real block as absent, turning a confirmed transaction into an unconfirmed one with no error surfaced anywhere, i.e. a silently wrong balance. The recovery path already exists: leave the height unresolved and retry it, which is what `unresolved_queries` is for. Driver side: `LocalChain::run_task` rejected `TaskProgress::Blocked` with `unreachable!()`. That was right about the outcome — this driver resolves every height a task announces via `Query` before polling again, so a task that announces what it needs never has anything outstanding — but it gave the reason as a property of the driver rather than of the task, and the panic was undocumented. Reaching that arm means the task is waiting on heights it never announced. Such a task would also hang a streaming driver, which starts fetches only for announced heights, so serving it by draining `unresolved_queries()` would hide the bug behind the one driver everyone tests against. Panic with a message that names the violation, document it under `# Panics`, and cover it with a test. Hand-off: phase 2 (`CanonicalViewTask`) is built from phase 1's `BlockQueries`, which may still hold pending heights, e.g. when phase 1 ran on another driver. Phase 2 would then wait on heights it never announced, the exact violation `run_task` now panics on. Add `BlockQueries::clear_pending` and call it when phase 2 takes over, so it reuses only resolved blocks and requests anything else itself. Cover it with a `BlockQueries` case and a phase-2 test. A streaming driver may still deliver phase 1's in-flight answers after the hand-off, so let `BlockQueries::resolve` accept answers for heights that are no longer pending instead of `debug_assert!`ing that they were. Co-Authored-By: Claude Opus 5 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01NND8BhEmYTYdLRv4dbUUro --- crates/chain/src/canonical.rs | 5 ++- crates/chain/src/canonical_view_task.rs | 5 ++- crates/chain/src/local_chain.rs | 22 ++++++++-- crates/chain/tests/test_canonical_view_mtp.rs | 22 +++++++++- crates/chain/tests/test_local_chain.rs | 41 +++++++++++++++++++ crates/core/src/block_queries.rs | 33 ++++++++++++--- crates/core/src/chain_task.rs | 19 +++++++-- 7 files changed, 130 insertions(+), 17 deletions(-) diff --git a/crates/chain/src/canonical.rs b/crates/chain/src/canonical.rs index a0fa612e1a..432afc7d7f 100644 --- a/crates/chain/src/canonical.rs +++ b/crates/chain/src/canonical.rs @@ -690,8 +690,9 @@ impl CanonicalTxs { /// Creates a [`CanonicalViewTask`] that resolves /// [`CanonicalReason`](crate::canonical_task::CanonicalReason)s into [`ChainPosition`]s. /// - /// This is the second phase of the canonicalization pipeline. Blocks fetched during - /// phase 1 are passed through so they can be reused without redundant queries. + /// This is the second phase of the canonicalization pipeline. Blocks resolved during + /// phase 1 are reused without redundant queries; requests still outstanding in `queries` are + /// dropped and made again if needed. /// /// To also compute median-time-past (MTP) values for the resulting view, call /// [`CanonicalViewTask::with_mtp`] on the returned task. diff --git a/crates/chain/src/canonical_view_task.rs b/crates/chain/src/canonical_view_task.rs index 774efef6b0..798c708571 100644 --- a/crates/chain/src/canonical_view_task.rs +++ b/crates/chain/src/canonical_view_task.rs @@ -65,8 +65,11 @@ impl<'g, A: Anchor, B> CanonicalViewTask<'g, A, B> { order: Vec, txs: HashMap>>, spends: HashMap, - queries: BlockQueries, + mut queries: BlockQueries, ) -> Self { + // Reuse only resolved blocks. A height phase 1 left in flight is requested again if + // needed, so this task only ever waits on heights it announced itself. + queries.clear_pending(); let all_anchors = tx_graph.all_anchors(); let mut unprocessed_anchor_checks = VecDeque::new(); diff --git a/crates/chain/src/local_chain.rs b/crates/chain/src/local_chain.rs index 13315846ee..2bd88493f7 100644 --- a/crates/chain/src/local_chain.rs +++ b/crates/chain/src/local_chain.rs @@ -434,6 +434,13 @@ where /// against this chain and returning the task's output. The chain responds with its data /// type `D`, so `LocalChain
` responds with headers and `LocalChain` /// responds with hashes. + /// + /// # Panics + /// + /// If the task returns [`TaskProgress::Blocked`]. This driver resolves every height a task + /// announces via [`TaskProgress::Query`] before polling it again, so a task that follows the + /// [`ChainTask`] contract never has anything outstanding here and never blocks. Reaching + /// this means the task is waiting on heights it never announced. pub fn run_task(&self, mut task: T) -> T::Output where T: ChainTask, @@ -456,11 +463,18 @@ where task.resolve_query(height, data); } } - // This is a synchronous driver: every `Query` height is resolved before the next - // poll, so the task never has an in-flight query and cannot return this variant. + // Every `Query` height is resolved before the next poll, so a task that announces + // what it needs never has anything outstanding here — `Blocked` means it is + // waiting on heights it never put in a `Query`. + // + // Serving that anyway (by draining `unresolved_queries()`) would be the wrong + // kindness: it hides the bug behind the one driver everyone tests against, while a + // streaming driver — which launches fetches only for announced heights — waits + // forever on I/O that was never started. Fail here, where it is cheap to find. TaskProgress::Blocked => { - unreachable!( - "run_task resolves queries synchronously; nothing is ever in-flight" + panic!( + "ChainTask returned Blocked to a synchronous driver: it is waiting on \ + heights it never announced via TaskProgress::Query" ) } } diff --git a/crates/chain/tests/test_canonical_view_mtp.rs b/crates/chain/tests/test_canonical_view_mtp.rs index 0b53dc07cb..dbf3d6545f 100644 --- a/crates/chain/tests/test_canonical_view_mtp.rs +++ b/crates/chain/tests/test_canonical_view_mtp.rs @@ -5,7 +5,10 @@ use std::collections::BTreeMap; -use bdk_chain::{local_chain::LocalChain, ChainPosition, ConfirmationBlockTime, TxGraph}; +use bdk_chain::{ + local_chain::LocalChain, BlockQueries, CanonicalTask, ChainPosition, ConfirmationBlockTime, + TxGraph, +}; use bdk_testenv::{hash, utils::new_tx}; use bitcoin::{ block::Header, hashes::Hash, Amount, BlockHash, OutPoint, ScriptBuf, Transaction, TxIn, TxOut, @@ -213,3 +216,20 @@ fn sparse_chain_yields_no_mtp_but_still_canonicalizes() { assert_eq!(txo.prev_mtp, None); assert_eq!(view.tip_mtp(), None); } + +/// Phase 2 drops requests still outstanding in the `BlockQueries` it is handed (e.g. left by a +/// phase 1 run on another driver), and requests them again itself. Otherwise it would wait on a +/// height `run_task` never saw in a `Query`, and `run_task` would panic on `Blocked`. +#[test] +fn view_task_requests_again_heights_left_pending() { + let chain = chain_with_time_equal_to_height(12); + let tx_graph = TxGraph::::default(); + let tip = chain.tip().block_id(); + + let (txs, _) = chain.run_task(CanonicalTask::new(&tx_graph, tip, Default::default())); + let mut queries = BlockQueries::new(); + let _ = queries.request([tip.height].into_iter()); + + let view = chain.run_task(txs.view_task(&tx_graph, queries).with_mtp()); + assert_eq!(view.tip_mtp(), Some(7)); +} diff --git a/crates/chain/tests/test_local_chain.rs b/crates/chain/tests/test_local_chain.rs index e6758559d9..8f614113f0 100644 --- a/crates/chain/tests/test_local_chain.rs +++ b/crates/chain/tests/test_local_chain.rs @@ -1435,3 +1435,44 @@ fn merge_chains_with_prev_blockhash() { .into_iter() .for_each(TestLocalChain::run); } + +/// A [`ChainTask`] that reports [`TaskProgress::Blocked`] without ever having announced the +/// height it is waiting on in a [`TaskProgress::Query`]. +/// +/// This is the only way a task reaches `run_task`'s `Blocked` arm: the driver resolves every +/// announced height before polling again, so a task that announces what it needs never has +/// anything outstanding. Such a task would also hang a streaming driver, which starts fetches +/// only for announced heights, so the synchronous driver panics rather than covering for it. +struct BlockedTask { + tip: BlockId, +} + +impl bdk_chain::ChainTask for BlockedTask { + type Output = (); + + fn tip(&self) -> BlockId { + self.tip + } + + fn poll(&mut self) -> bdk_chain::TaskProgress { + bdk_chain::TaskProgress::Blocked + } + + fn resolve_query(&mut self, _height: u32, _response: Option) {} + + fn unresolved_queries<'a>(&'a self) -> impl Iterator + 'a { + core::iter::empty() + } + + fn finish(self) -> Self::Output {} +} + +#[test] +#[should_panic(expected = "heights it never announced")] +fn run_task_panics_when_a_task_blocks_without_querying() { + let chain = local_chain![(0, hash!("_")), (1, hash!("A"))]; + + chain.run_task(BlockedTask { + tip: chain.tip().block_id(), + }); +} diff --git a/crates/core/src/block_queries.rs b/crates/core/src/block_queries.rs index 334161f587..f73bc1846e 100644 --- a/crates/core/src/block_queries.rs +++ b/crates/core/src/block_queries.rs @@ -41,13 +41,17 @@ impl BlockQueries { } /// Marks a height as resolved. Removes from pending and inserts into blocks. + /// + /// `block` is `None` when the chain source **has no block** at `height`. This is terminal: + /// the height leaves [`unresolved`](Self::unresolved) and [`request`](Self::request) will + /// never return it again. So do not pass `None` for a *failed* fetch; leave the height + /// pending and retry it, or the failure is silently recorded as "no such block". + /// + /// `height` need not be pending: after [`clear_pending`](Self::clear_pending), an answer to a + /// dropped request, or a second answer to one requested again, is recorded like any other. pub fn resolve(&mut self, height: u32, block: Option) { - let was_pending = self.pending.remove(&height); + self.pending.remove(&height); self.blocks.insert(height, block); - debug_assert!( - was_pending, - "request must be previously unresolved to be resolved now" - ); } /// Iterates over heights that are still pending (unresolved). @@ -55,6 +59,15 @@ impl BlockQueries { self.pending.iter().copied() } + /// Drops every pending request, keeping only resolved blocks. + /// + /// Call this when handing a `BlockQueries` to a new task, so that every height the new task + /// waits on is one it requested itself. A dropped height is returned by + /// [`request`](Self::request) again. + pub fn clear_pending(&mut self) { + self.pending.clear(); + } + /// Looks up a resolved block by height. pub fn get(&self, height: u32) -> Option<&Option> { self.blocks.get(&height) @@ -205,6 +218,16 @@ mod test { expected: BlockCandidateResolution::Awaiting, expected_unresolved: vec![7], }, + Case { + name: "query again for a height dropped by clear_pending", + setup: |q| { + q.request([7].into_iter()); + q.clear_pending(); + }, + candidates: vec![id(7, 7)], + expected: BlockCandidateResolution::Query(vec![7]), + expected_unresolved: vec![7], + }, Case { name: "not confirmed when the block is resolved but mismatched", setup: |q| { diff --git a/crates/core/src/chain_task.rs b/crates/core/src/chain_task.rs index a65b1ef987..7dfc24f66b 100644 --- a/crates/core/src/chain_task.rs +++ b/crates/core/src/chain_task.rs @@ -113,14 +113,25 @@ pub trait ChainTask { /// Provides the block data (or `None`) for a previously requested `height`. /// - /// A response of `None` signals that the chain source has no block at that - /// height. The task records this so it can distinguish "not yet queried" - /// from "queried but absent." + /// A response of `None` signals that the chain source **has no block** at that height: + /// it is an answer, not an error. The task records it so it can distinguish "not yet + /// queried" from "queried but absent", and treats every anchor at that height as not + /// in-chain. + /// + /// Resolving is terminal: a height is answered once, and the task will not ask for it + /// again. A driver that *failed* to fetch a height (I/O error, timeout) must therefore + /// **not** call this with `None`: that would silently record "no such block" and, for an + /// anchor that is really in-chain, downgrade a confirmed transaction to unconfirmed with + /// no error surfaced anywhere. Leave the height unresolved instead and retry it; see + /// [`unresolved_queries`](Self::unresolved_queries). fn resolve_query(&mut self, height: u32, response: Option); /// Returns heights that have been requested but not yet resolved. /// - /// This is useful for retrying failed queries or reporting progress. + /// This is the retry set: a height a fetch failed for stays here until the driver + /// resolves it, so a driver can re-issue those fetches (and report progress) after an + /// error. Once [`resolve_query`](Self::resolve_query) is called for a height it leaves + /// this set permanently. fn unresolved_queries<'a>(&'a self) -> impl Iterator + 'a; /// Consumes the task and returns the final output. From a7d75c58aad3899c0e77338171f0c21b8b3c5e87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Thu, 10 Sep 2026 11:03:38 +0000 Subject: [PATCH 6/9] feat(core): derive `Debug` for `BlockQueries` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `BlockQueries` is half of `CanonicalTask::Output`, so without `Debug` the task's own output tuple isn't `Debug` — `dbg!(chain.run_task(task))` and `.unwrap()` on a `Result` in that position don't compile. Its neighbours `BlockCandidateResolution` and `Canonical` already derive it (Rust API guideline C-DEBUG). Covered by formatting the output tuple in the existing `CanonicalTask` test, so the derive can't be dropped without a test failing. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NND8BhEmYTYdLRv4dbUUro --- crates/chain/src/canonical_task.rs | 6 +++++- crates/core/src/block_queries.rs | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/chain/src/canonical_task.rs b/crates/chain/src/canonical_task.rs index 93d190e7dd..2b6649e860 100644 --- a/crates/chain/src/canonical_task.rs +++ b/crates/chain/src/canonical_task.rs @@ -501,7 +501,11 @@ mod tests { // Create canonicalization task and canonicalize using the two-step pipeline let params = CanonicalParams::default(); let task = CanonicalTask::new(&tx_graph, chain_tip, params); - let (txs, queries) = chain.run_task(task); + let output = chain.run_task(task); + // `CanonicalTask::Output` is `(CanonicalTxs, BlockQueries)`. Both halves must be + // `Debug` or callers cannot `dbg!()` it, or `.unwrap()` it out of a `Result`. + let _ = alloc::format!("{output:?}"); + let (txs, queries) = output; let view_task = txs.view_task(&tx_graph, queries); let canonical_view = chain.run_task(view_task); diff --git a/crates/core/src/block_queries.rs b/crates/core/src/block_queries.rs index f73bc1846e..8d40db113e 100644 --- a/crates/core/src/block_queries.rs +++ b/crates/core/src/block_queries.rs @@ -17,6 +17,7 @@ use alloc::vec::Vec; /// 2. Feed responses back via [`resolve`](Self::resolve). /// 3. Look up resolved blocks with [`get`](Self::get). /// 4. Check for outstanding requests with [`unresolved`](Self::unresolved). +#[derive(Debug)] pub struct BlockQueries { pending: BTreeSet, blocks: BTreeMap>, From 8e10dfa3f4098ea07ad9c997880e27e1ab1cc2dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Fri, 11 Sep 2026 03:36:51 +0000 Subject: [PATCH 7/9] fix(chain): report the best-known mempool `last_seen`, not a lossy one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ChainPosition::Unconfirmed::last_seen` is documented as `None` only when the transaction "was never seen in the mempool **and** only seen in a conflicting chain". Both arms of the `ObservedIn` match discarded a real sighting that contradicted that. `TxGraph::txids_by_descending_last_seen` filters evicted transactions out, so an evicted transaction never reaches the mempool stage of canonicalization. It can still be marked canonical *transitively*, swept by `mark_canonical` as the ancestor of a canonical descendant — and it arrives with a `last_seen` the stage never got to consider. `ObservedIn::Block` forced `None`. That gave `first_seen: Some(t), last_seen: None` — "first seen at t, never seen" — since `first_seen` is passed through on exactly the same terms one line above. Report `tx_node.last_seen` instead. `ObservedIn::Mempool` used the descendant's sighting alone. That is only a lower bound (a child cannot sit in the mempool without its parent), so it discarded a later direct observation in favour of an inferred one, and could likewise place `last_seen` before `first_seen`. Take the later of the two. Neither changes the ordinary path: a transaction with a sighting that has not been evicted is marked in the mempool stage, which runs newest-first, so a sweep only ever reaches an ancestor whose own sighting is older. That is why both were invisible. Adds a regression test per arm; nothing covered the evicted-ancestor path. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NND8BhEmYTYdLRv4dbUUro --- crates/chain/src/canonical_view_task.rs | 23 ++- crates/chain/tests/test_canonical_view.rs | 164 ++++++++++++++++++++++ 2 files changed, 181 insertions(+), 6 deletions(-) diff --git a/crates/chain/src/canonical_view_task.rs b/crates/chain/src/canonical_view_task.rs index 798c708571..e0f377e6ab 100644 --- a/crates/chain/src/canonical_view_task.rs +++ b/crates/chain/src/canonical_view_task.rs @@ -332,13 +332,24 @@ impl<'g, A: Anchor, B: ToBlockHash> ChainTask for CanonicalViewTask<'g, A, B> }, CanonicalReason::ObservedIn { observed_in, .. } => ChainPosition::Unconfirmed { first_seen: tx_node.first_seen, - // `last_seen` is a mempool sighting. `ObservedIn::Block` means the tx was - // only ever seen in a block that is not in the best chain, so there is no - // mempool sighting to report — a tx that re-entered the mempool would be - // `ObservedIn::Mempool` instead. + // `ObservedIn::Block` usually means no mempool sighting, but not always: + // `txids_by_descending_last_seen` filters out evicted txs, so an evicted tx + // never reaches the mempool stage and can arrive here transitively, as the + // ancestor of a stale-anchored tx, while genuinely having a `last_seen`. + // Report it — `last_seen` is documented as `None` only when the tx was never + // seen in the mempool, and `first_seen` above is passed through on the same + // terms. last_seen: match observed_in { - ObservedIn::Block(_) => None, - ObservedIn::Mempool(seen_at) => Some(*seen_at), + ObservedIn::Block(_) => tx_node.last_seen, + // For a transitively-marked ancestor this is the *descendant's* + // sighting, which is only a lower bound: a child cannot sit in the + // mempool without its parent. Take whichever is later, so a direct + // observation of this tx is never discarded in favour of an inferred + // one. `None` sorts below `Some`, so this is `seen_at` when the tx has + // no sighting of its own. Ordinarily they agree — the mempool stage runs + // newest-first, so a sweep only reaches an ancestor whose own sighting is + // older — but evicted txs are filtered out of that stage. + ObservedIn::Mempool(seen_at) => Some(*seen_at).max(tx_node.last_seen), }, }, }; diff --git a/crates/chain/tests/test_canonical_view.rs b/crates/chain/tests/test_canonical_view.rs index 2672c56688..32455154e7 100644 --- a/crates/chain/tests/test_canonical_view.rs +++ b/crates/chain/tests/test_canonical_view.rs @@ -1182,3 +1182,167 @@ fn test_classify_locked_descriptor_timelocks() { assert_eq!(classify(&chain, &tx_graph), expected, "tip {tip}"); } } + +/// An evicted transaction swept transitively as the ancestor of a stale-anchored transaction must +/// still report the mempool sighting it genuinely has. +/// +/// `ChainPosition::Unconfirmed::last_seen` is documented as `None` only when the transaction was +/// never seen in the mempool *and* only seen in a conflicting chain. An evicted transaction fails +/// the first condition (it has a real `last_seen`), but +/// `TxGraph::txids_by_descending_last_seen` filters evicted transactions out, so it never reaches +/// the mempool stage of canonicalization and instead arrives via `ObservedIn::Block`. Forcing +/// `None` there would report `first_seen: Some(_), last_seen: None`, i.e. "first seen at t, never +/// seen", since `first_seen` is passed through unconditionally. +#[test] +fn test_evicted_ancestor_of_stale_anchored_tx_keeps_last_seen() { + let blocks: BTreeMap = + [(0, hash!("genesis")), (1, hash!("b1")), (2, hash!("tip"))] + .into_iter() + .collect(); + let chain = LocalChain::from_blocks(blocks).unwrap(); + + let mut tx_graph = TxGraph::default(); + + // Parent: seen in the mempool at 100, then evicted at 200. + let parent = Transaction { + input: vec![TxIn { + previous_output: OutPoint::new(hash!("grandparent"), 0), + ..Default::default() + }], + output: vec![TxOut { + value: Amount::from_sat(50_000), + script_pubkey: ScriptBuf::new(), + }], + ..new_tx(1) + }; + let parent_txid = parent.compute_txid(); + let _ = tx_graph.insert_tx(parent.clone()); + let _ = tx_graph.insert_seen_at(parent_txid, 100); + let _ = tx_graph.insert_evicted_at(parent_txid, 200); + + // Child: spends the parent and is anchored only in a block that is not in the best chain, so + // it is canonicalized in the leftover stage as `ObservedIn::Block`. It has no mempool + // sighting of its own, so it is not evicted and the leftover stage does not skip it. + let child = Transaction { + input: vec![TxIn { + previous_output: OutPoint::new(parent_txid, 0), + ..Default::default() + }], + output: vec![TxOut { + value: Amount::from_sat(40_000), + script_pubkey: ScriptBuf::new(), + }], + ..new_tx(2) + }; + let child_txid = child.compute_txid(); + let _ = tx_graph.insert_tx(child); + let _ = tx_graph.insert_anchor( + child_txid, + ConfirmationBlockTime { + block_id: BlockId { + height: 1, + hash: hash!("stale"), + }, + confirmation_time: 123456, + }, + ); + + let view = chain.canonicalize(&tx_graph, chain.tip().block_id(), Default::default()); + + let parent_pos = view + .txs() + .find(|c| c.txid == parent_txid) + .expect("parent is swept canonical as an ancestor of the child") + .pos; + + match parent_pos { + ChainPosition::Unconfirmed { + first_seen, + last_seen, + } => { + assert_eq!(first_seen, Some(100), "first_seen is passed through"); + assert_eq!( + last_seen, + Some(100), + "an evicted tx was seen in the mempool, so its last_seen must be reported" + ); + } + pos => panic!("parent must be unconfirmed, got {pos:?}"), + } +} + +/// A transitively-marked ancestor must not lose a later mempool sighting of its own. +/// +/// `ObservedIn::Mempool` carries the *descendant's* `last_seen`, which is only an inference: a +/// child cannot sit in the mempool without its parent. `TxGraph::txids_by_descending_last_seen` +/// iterates newest-first and `mark_canonical` skips anything already canonical, so a sweep +/// normally only reaches an ancestor whose own sighting is older. Evicted transactions are +/// filtered out of that iteration, though, so an evicted ancestor can be swept with a timestamp +/// older than the one we directly observed for it. +#[test] +fn test_evicted_ancestor_keeps_its_own_later_last_seen() { + let blocks: BTreeMap = [(0, hash!("genesis")), (1, hash!("tip"))] + .into_iter() + .collect(); + let chain = LocalChain::from_blocks(blocks).unwrap(); + + let mut tx_graph = TxGraph::::default(); + + // Parent: directly observed in the mempool at 300, then evicted at 400. The eviction keeps + // it out of the mempool stage, so it is only ever marked canonical transitively. + let parent = Transaction { + input: vec![TxIn { + previous_output: OutPoint::new(hash!("grandparent"), 0), + ..Default::default() + }], + output: vec![TxOut { + value: Amount::from_sat(50_000), + script_pubkey: ScriptBuf::new(), + }], + ..new_tx(1) + }; + let parent_txid = parent.compute_txid(); + let _ = tx_graph.insert_tx(parent); + let _ = tx_graph.insert_seen_at(parent_txid, 300); + let _ = tx_graph.insert_evicted_at(parent_txid, 400); + + // Child: spends the parent, seen at 200 — *earlier* than the parent's own sighting. + let child = Transaction { + input: vec![TxIn { + previous_output: OutPoint::new(parent_txid, 0), + ..Default::default() + }], + output: vec![TxOut { + value: Amount::from_sat(40_000), + script_pubkey: ScriptBuf::new(), + }], + ..new_tx(2) + }; + let child_txid = child.compute_txid(); + let _ = tx_graph.insert_tx(child); + let _ = tx_graph.insert_seen_at(child_txid, 200); + + let view = chain.canonicalize(&tx_graph, chain.tip().block_id(), Default::default()); + + let parent_pos = view + .txs() + .find(|c| c.txid == parent_txid) + .expect("parent is swept canonical as an ancestor of the child") + .pos; + + match parent_pos { + ChainPosition::Unconfirmed { + first_seen, + last_seen, + } => { + assert_eq!(first_seen, Some(300)); + assert_eq!( + last_seen, + Some(300), + "the parent's own sighting is later than the child's, so it must win; \ + reporting the child's 200 would also put last_seen before first_seen" + ); + } + pos => panic!("parent must be unconfirmed, got {pos:?}"), + } +} From 333f0818ed75a009bc45a99c820be39711fe4cbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 20 Oct 2026 01:25:08 +1100 Subject: [PATCH 8/9] refactor(core,chain)!: pin `BlockQueries` to a chain tip `BlockQueries` stored answers by height alone, while each `ChainTask` kept its own `tip`. Now that `resolve` accepts answers for heights that are no longer pending, nothing said which chain those answers belong to: a second answer for a height could come from a different chain (e.g. after a reorg) and silently replace the first, after a task had already acted on it. Make `BlockQueries` carry the tip its answers are relative to (`BlockQueries::new(tip)`, `BlockQueries::tip`), and have `CanonicalTask` and `CanonicalViewTask` read their tip from it instead of storing their own. Every answer is then the block at that height in the chain ending at `tip`, so a height has one right answer however often it is fetched. Drop `Default`, as there is no default tip. `CanonicalTxs::view_task` reuses the `BlockQueries` it is handed only if it is for the same tip as phase 1; otherwise phase 2 starts empty and fetches what it needs, rather than trusting blocks from another chain. Co-Authored-By: Claude Opus 5.5 --- crates/chain/src/canonical.rs | 18 ++++++------ crates/chain/src/canonical_task.rs | 8 ++---- crates/chain/src/canonical_view_task.rs | 17 ++++++----- crates/chain/tests/test_canonical_view_mtp.rs | 18 +++++++++++- crates/core/src/block_queries.rs | 28 +++++++++++-------- 5 files changed, 56 insertions(+), 33 deletions(-) diff --git a/crates/chain/src/canonical.rs b/crates/chain/src/canonical.rs index 432afc7d7f..74ea67e4b4 100644 --- a/crates/chain/src/canonical.rs +++ b/crates/chain/src/canonical.rs @@ -692,7 +692,8 @@ impl CanonicalTxs { /// /// This is the second phase of the canonicalization pipeline. Blocks resolved during /// phase 1 are reused without redundant queries; requests still outstanding in `queries` are - /// dropped and made again if needed. + /// dropped and made again if needed. If `queries` is for a different tip than this set, its + /// blocks are not reused at all. /// /// To also compute median-time-past (MTP) values for the resulting view, call /// [`CanonicalViewTask::with_mtp`] on the returned task. @@ -701,13 +702,12 @@ impl CanonicalTxs { tx_graph: &'g TxGraph, queries: BlockQueries, ) -> CanonicalViewTask<'g, A, B> { - CanonicalViewTask::new( - tx_graph, - self.tip, - self.order, - self.txs, - self.spends, - queries, - ) + // Blocks fetched for another tip may not be in this tip's chain. + let queries = if queries.tip() == self.tip { + queries + } else { + BlockQueries::new(self.tip) + }; + CanonicalViewTask::new(tx_graph, self.order, self.txs, self.spends, queries) } } diff --git a/crates/chain/src/canonical_task.rs b/crates/chain/src/canonical_task.rs index 2b6649e860..258ca8d4a8 100644 --- a/crates/chain/src/canonical_task.rs +++ b/crates/chain/src/canonical_task.rs @@ -60,7 +60,6 @@ pub struct CanonicalParams { /// into [`ChainPosition`](crate::ChainPosition)s. pub struct CanonicalTask<'g, A, B> { tx_graph: &'g TxGraph, - chain_tip: BlockId, queries: BlockQueries, @@ -83,7 +82,7 @@ impl<'g, A: Anchor, B: ToBlockHash> ChainTask for CanonicalTask<'g, A, B> { type Output = (CanonicalTxs, BlockQueries); fn tip(&self) -> BlockId { - self.chain_tip + self.queries.tip() } fn unresolved_queries<'a>(&'a self) -> impl Iterator + 'a { @@ -215,7 +214,7 @@ impl<'g, A: Anchor, B: ToBlockHash> ChainTask for CanonicalTask<'g, A, B> { ( CanonicalTxs::new( - self.chain_tip, + self.queries.tip(), self.canonical_order, self.canonical, view_spends, @@ -249,7 +248,6 @@ impl<'g, A: Anchor, B: ToBlockHash> CanonicalTask<'g, A, B> { Self { tx_graph, - chain_tip, unprocessed_assumed_txs, unprocessed_anchored_txs, @@ -262,7 +260,7 @@ impl<'g, A: Anchor, B: ToBlockHash> CanonicalTask<'g, A, B> { canonical_order: Vec::new(), current_stage: CanonicalStage::default(), - queries: BlockQueries::new(), + queries: BlockQueries::new(chain_tip), } } diff --git a/crates/chain/src/canonical_view_task.rs b/crates/chain/src/canonical_view_task.rs index e0f377e6ab..62118447d6 100644 --- a/crates/chain/src/canonical_view_task.rs +++ b/crates/chain/src/canonical_view_task.rs @@ -38,7 +38,6 @@ enum ViewStage { /// for confirmed heights and stores them in the resulting [`CanonicalView`]. pub struct CanonicalViewTask<'g, A, B> { tx_graph: &'g TxGraph, - tip: BlockId, queries: BlockQueries, @@ -61,7 +60,6 @@ impl<'g, A: Anchor, B> CanonicalViewTask<'g, A, B> { /// already fetched during phase 1 (to avoid redundant queries). pub(crate) fn new( tx_graph: &'g TxGraph, - tip: BlockId, order: Vec, txs: HashMap>>, spends: HashMap, @@ -107,7 +105,6 @@ impl<'g, A: Anchor, B> CanonicalViewTask<'g, A, B> { Self { tx_graph, - tip, queries, canonical_order: order, canonical_txs: txs, @@ -157,7 +154,7 @@ impl<'g, A: Anchor, B: ToBlockHash> ChainTask for CanonicalViewTask<'g, A, B> type Output = CanonicalView; fn tip(&self) -> BlockId { - self.tip + self.queries.tip() } fn unresolved_queries<'a>(&'a self) -> impl Iterator + 'a { @@ -210,7 +207,7 @@ impl<'g, A: Anchor, B: ToBlockHash> ChainTask for CanonicalViewTask<'g, A, B> // is rebuilt on every poll of this stage. It also makes the emitted `Query` // ascending and deterministic — `direct_anchors` is a `HashMap`, so iterating its // values would otherwise vary the height order between runs. - let required: BTreeSet = core::iter::once(self.tip.height) + let required: BTreeSet = core::iter::once(self.queries.tip().height) .chain( self.direct_anchors .values() @@ -256,7 +253,7 @@ impl<'g, A: Anchor, B: ToBlockHash> ChainTask for CanonicalViewTask<'g, A, B> }; // Compute tip MTP. - let tip_mtp = try_compute_mtp_at(self.tip.height); + let tip_mtp = try_compute_mtp_at(self.queries.tip().height); let mut view_order = Vec::new(); let mut view_txs = HashMap::>::new(); @@ -367,6 +364,12 @@ impl<'g, A: Anchor, B: ToBlockHash> ChainTask for CanonicalViewTask<'g, A, B> ); } - CanonicalView::new(self.tip, view_order, view_txs, self.spends, tip_mtp) + CanonicalView::new( + self.queries.tip(), + view_order, + view_txs, + self.spends, + tip_mtp, + ) } } diff --git a/crates/chain/tests/test_canonical_view_mtp.rs b/crates/chain/tests/test_canonical_view_mtp.rs index dbf3d6545f..84c83d6f1a 100644 --- a/crates/chain/tests/test_canonical_view_mtp.rs +++ b/crates/chain/tests/test_canonical_view_mtp.rs @@ -227,9 +227,25 @@ fn view_task_requests_again_heights_left_pending() { let tip = chain.tip().block_id(); let (txs, _) = chain.run_task(CanonicalTask::new(&tx_graph, tip, Default::default())); - let mut queries = BlockQueries::new(); + let mut queries = BlockQueries::new(tip); let _ = queries.request([tip.height].into_iter()); let view = chain.run_task(txs.view_task(&tx_graph, queries).with_mtp()); assert_eq!(view.tip_mtp(), Some(7)); } + +/// Phase 2 does not reuse blocks fetched for a different tip, since they may not be in its chain. +#[test] +fn view_task_ignores_queries_for_another_tip() { + let chain = chain_with_time_equal_to_height(12); + let tx_graph = TxGraph::::default(); + let tip = chain.tip().block_id(); + + let (txs, _) = chain.run_task(CanonicalTask::new(&tx_graph, tip, Default::default())); + let mut queries = BlockQueries::new(chain.get(11).expect("height 11 exists").block_id()); + let _ = queries.request([tip.height].into_iter()); + queries.resolve(tip.height, None); + + let view = chain.run_task(txs.view_task(&tx_graph, queries).with_mtp()); + assert_eq!(view.tip_mtp(), Some(7)); +} diff --git a/crates/core/src/block_queries.rs b/crates/core/src/block_queries.rs index 8d40db113e..67995feb6d 100644 --- a/crates/core/src/block_queries.rs +++ b/crates/core/src/block_queries.rs @@ -6,6 +6,9 @@ use alloc::vec::Vec; /// Tracks block-height queries: which heights have been requested, /// which are still pending, and the resolved data. /// +/// Every answer is for the chain ending at [`tip`](Self::tip), so a height has only one right +/// answer, however many times it is fetched. +/// /// This is a helper for [`ChainTask`](crate::ChainTask) implementors that need to /// track which block heights have been requested from the chain source and which /// responses have been received. It deduplicates requests and stores resolved blocks. @@ -19,19 +22,26 @@ use alloc::vec::Vec; /// 4. Check for outstanding requests with [`unresolved`](Self::unresolved). #[derive(Debug)] pub struct BlockQueries { + tip: BlockId, pending: BTreeSet, blocks: BTreeMap>, } impl BlockQueries { - /// Creates an empty `BlockQueries`. - pub fn new() -> Self { + /// Creates an empty `BlockQueries` for the chain ending at `tip`. + pub fn new(tip: BlockId) -> Self { Self { + tip, pending: BTreeSet::new(), blocks: BTreeMap::new(), } } + /// The chain tip that every answer is relative to. + pub fn tip(&self) -> BlockId { + self.tip + } + /// Filters out already-resolved/pending heights, inserts new ones into pending, /// and returns the list of newly requested heights. pub fn request(&mut self, heights: impl Iterator) -> Vec { @@ -48,8 +58,10 @@ impl BlockQueries { /// never return it again. So do not pass `None` for a *failed* fetch; leave the height /// pending and retry it, or the failure is silently recorded as "no such block". /// - /// `height` need not be pending: after [`clear_pending`](Self::clear_pending), an answer to a - /// dropped request, or a second answer to one requested again, is recorded like any other. + /// `block` must be the block at `height` in the chain ending at [`tip`](Self::tip), so a + /// second answer for a height repeats the first. `height` need not be pending: after + /// [`clear_pending`](Self::clear_pending), a late answer to a dropped request is recorded + /// like any other. pub fn resolve(&mut self, height: u32, block: Option) { self.pending.remove(&height); self.blocks.insert(height, block); @@ -126,12 +138,6 @@ impl BlockQueries { } } -impl Default for BlockQueries { - fn default() -> Self { - Self::new() - } -} - /// Outcome of [`BlockQueries::resolve_candidates`]. /// /// A "candidate" is some value `C` that claims to live at a particular [`BlockId`] (height + @@ -252,7 +258,7 @@ mod test { ]; for case in cases { - let mut q = BlockQueries::::new(); + let mut q = BlockQueries::::new(id(10, 10)); (case.setup)(&mut q); let res = q.resolve_candidates(case.candidates.iter().map(|id| (*id, *id))); From 471774821d0553a8d550dd3a6ccbc06a016cab55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Mon, 19 Oct 2026 15:36:48 +0000 Subject: [PATCH 9/9] feat(chain)!: check timelocks in the view, not the caller `classify_outpoints` and `balance` took an `is_locked` predicate, so every caller had to compare its script's timelocks against the chain itself: the tip height (spendable in block `tip + 1`), `tip_mtp` (BIP-113), `prev_mtp` (BIP-68) and the 512-second units, and remember that an unknown MTP means locked. Take a `timelocks` callback returning the spend path's `Timelocks` (an absolute and/or relative `LockTime`) instead, and do the comparison in the view with the new `CanonicalTxOut::are_timelocks_met`, as `is_mature` already does for coinbase maturity. A lock that needs an unknown MTP is unmet. An unconfirmed output is treated as confirming in the block after the tip, as Bitcoin Core does for mempool coins. Both methods are unreleased, so the signature change is free until the next chain release. Co-Authored-By: Claude Opus 5.5 --- crates/bitcoind_rpc/tests/test_emitter.rs | 2 +- crates/chain/benches/indexer.rs | 2 +- crates/chain/benches/trust_classification.rs | 18 +- crates/chain/src/canonical.rs | 86 ++++++-- crates/chain/tests/test_canonical_view.rs | 195 ++++++++++++++---- crates/chain/tests/test_indexed_tx_graph.rs | 2 +- crates/chain/tests/test_tx_graph_conflicts.rs | 2 +- crates/electrum/tests/test_electrum.rs | 2 +- 8 files changed, 241 insertions(+), 68 deletions(-) diff --git a/crates/bitcoind_rpc/tests/test_emitter.rs b/crates/bitcoind_rpc/tests/test_emitter.rs index ac4a3657c2..55980f0cf2 100644 --- a/crates/bitcoind_rpc/tests/test_emitter.rs +++ b/crates/bitcoind_rpc/tests/test_emitter.rs @@ -331,7 +331,7 @@ fn get_balance( outpoints.into_iter().map(|(_, op)| op), bdk_chain::taints_unowned(&recv_graph.index), |pos| pos.is_confirmed(), - |_txout| false, + |_txout| Default::default(), ); Ok(balance) } diff --git a/crates/chain/benches/indexer.rs b/crates/chain/benches/indexer.rs index ebc07ffb38..3b0faf2251 100644 --- a/crates/chain/benches/indexer.rs +++ b/crates/chain/benches/indexer.rs @@ -89,7 +89,7 @@ fn do_bench(indexed_tx_graph: &KeychainTxGraph, chain: &LocalChain) { op.into_iter().map(|(_, o)| o), bdk_chain::taints_unowned(&graph.index), |pos| pos.is_confirmed(), - |_txout| false, + |_txout| Default::default(), ); assert_eq!(bal.total(), AMOUNT * TX_CT as u64); } diff --git a/crates/chain/benches/trust_classification.rs b/crates/chain/benches/trust_classification.rs index 606690b755..578097c9d5 100644 --- a/crates/chain/benches/trust_classification.rs +++ b/crates/chain/benches/trust_classification.rs @@ -7,7 +7,8 @@ use std::collections::BTreeMap; use bdk_chain::{ - local_chain::LocalChain, CanonicalView, ChainPosition, ConfirmationBlockTime, TxGraph, + local_chain::LocalChain, CanonicalView, ChainPosition, ConfirmationBlockTime, Timelocks, + TxGraph, }; use bdk_testenv::{hash, utils::new_tx}; use bitcoin::{Amount, BlockHash, OutPoint, Script, ScriptBuf, Transaction, TxIn, TxOut, Txid}; @@ -23,7 +24,7 @@ fn make_chain() -> LocalChain { // The three predicates `classify_outpoints` takes, faking what a wallet would pass: // `does_taint` = the tx pulls in coins that aren't ours; `is_settled` = the tx is confirmed; -// `is_locked` = the tx has timelocked outputs. +// `timelocks` = no output is timelocked. #[allow(clippy::type_complexity)] fn classify_outpoints_predicates<'a>( view: &'a CanonicalView, @@ -31,7 +32,7 @@ fn classify_outpoints_predicates<'a>( ) -> ( impl FnMut(&bdk_chain::CanonicalTx>) -> bool + 'a, impl Fn(&ChainPosition) -> bool, - impl Fn(&bdk_chain::CanonicalTxOut>) -> bool, + impl Fn(&bdk_chain::CanonicalTxOut>) -> Timelocks, ) { let owned = owned.clone(); let is_mine = move |spk: &Script| spk == owned.as_script(); @@ -50,10 +51,11 @@ fn classify_outpoints_predicates<'a>( let is_settled = |pos: &ChainPosition| matches!(pos, ChainPosition::Confirmed { .. }); - let is_locked = - |_txout: &bdk_chain::CanonicalTxOut>| false; + let timelocks = |_txout: &bdk_chain::CanonicalTxOut>| { + Timelocks::default() + }; - (does_taint, is_settled, is_locked) + (does_taint, is_settled, timelocks) } // Per-UTXO memoized classification (`classify_outpoints`). @@ -63,8 +65,8 @@ fn run_classify( owned: &ScriptBuf, ) { let outpoints = utxo_txids.iter().map(|&txid| OutPoint::new(txid, 0)); - let (does_taint, is_settled, is_locked) = classify_outpoints_predicates(view, owned); - for item in view.classify_outpoints(outpoints, does_taint, is_settled, is_locked) { + let (does_taint, is_settled, timelocks) = classify_outpoints_predicates(view, owned); + for item in view.classify_outpoints(outpoints, does_taint, is_settled, timelocks) { std::hint::black_box(item); } } diff --git a/crates/chain/src/canonical.rs b/crates/chain/src/canonical.rs index 74ea67e4b4..08daad9864 100644 --- a/crates/chain/src/canonical.rs +++ b/crates/chain/src/canonical.rs @@ -27,7 +27,9 @@ use alloc::vec::Vec; use core::{fmt, ops::RangeBounds}; use bdk_core::{BlockId, BlockQueries}; -use bitcoin::{constants::COINBASE_MATURITY, OutPoint, ScriptBuf, Transaction, TxOut, Txid}; +use bitcoin::{ + absolute, constants::COINBASE_MATURITY, relative, OutPoint, ScriptBuf, Transaction, TxOut, Txid, +}; use crate::{spk_txout::SpkTxOutIndex, Anchor, Balance, CanonicalViewTask, ChainPosition, TxGraph}; @@ -39,14 +41,22 @@ pub enum Eligibility { /// [`classify_outpoints`](CanonicalView::classify_outpoints). Typically confirmed deeply /// enough to be unlikely to be replaced, but the caller decides. Settled, - /// An output not yet spendable by consensus: an immature coinbase output, or one the - /// `is_locked` predicate reports as timelocked. May be unconfirmed; its [`Trust`] is not - /// evaluated. + /// An output not yet spendable by consensus: an immature coinbase output, or one whose + /// [`Timelocks`] are not met. May be unconfirmed; its [`Trust`] is not evaluated. Immature, /// An output not yet settled. Unsettled(Trust), } +/// The timelocks of the spend path the caller will use for an output. A path can have both. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash)] +pub struct Timelocks { + /// The `nLockTime` the spending transaction needs (BIP-65, e.g. miniscript `after`). + pub absolute: Option, + /// The `nSequence` the spending input needs (BIP-112, e.g. miniscript `older`). + pub relative: Option, +} + /// Describes whether an [`Unsettled`](Eligibility::Unsettled) output is trusted, untrusted, or of /// unknown trust because the `CanonicalView` doesn't have its full ancestry. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -177,6 +187,50 @@ impl CanonicalTxOut> { !self.is_on_coinbase || self.pos.confirmations_lower_bound(tip) >= COINBASE_MATURITY } + /// Whether `timelocks` are met for a transaction spending this output in the block after + /// `tip`, where `tip_mtp` is the MTP at `tip` (see [`Canonical::tip_mtp`]). + /// + /// This follows consensus (BIP-65, BIP-68, BIP-113). An unconfirmed output is treated as + /// confirming in the block after `tip`, as Bitcoin Core does for mempool coins. A lock that + /// needs an unknown MTP (`None` [`prev_mtp`](Self::prev_mtp) or `tip_mtp`) counts as unmet. + /// + /// If the position is confirmed [`transitively`](ChainPosition::Confirmed::transitively), the + /// number of confirmations may be a lower bound (see [`confirmations_lower_bound`]), so this + /// method may return false-negatives. + /// + /// [`confirmations_lower_bound`]: ChainPosition::confirmations_lower_bound + pub fn are_timelocks_met(&self, timelocks: Timelocks, tip: u32, tip_mtp: Option) -> bool { + let next_height = tip.saturating_add(1); + let absolute_met = match timelocks.absolute { + None => true, + Some(absolute::LockTime::Blocks(h)) => h.to_consensus_u32() < next_height, + Some(absolute::LockTime::Seconds(t)) => { + tip_mtp.is_some_and(|mtp| t.to_consensus_u32() < mtp) + } + }; + let relative_met = match timelocks.relative { + None => true, + // An unconfirmed output has 0 confirmations: as if it confirms in `next_height`. + Some(relative::LockTime::Blocks(n)) => { + self.pos.confirmations_lower_bound(tip) >= u32::from(n.value()) + } + Some(relative::LockTime::Time(n)) => { + let coin_mtp = if self.pos.is_confirmed() { + self.prev_mtp + } else { + tip_mtp + }; + match (coin_mtp, tip_mtp) { + (Some(coin_mtp), Some(tip_mtp)) => { + u64::from(coin_mtp) + u64::from(n.value()) * 512 <= u64::from(tip_mtp) + } + _ => false, + } + } + }; + absolute_met && relative_met + } + /// Whether the utxo is/was/will be spendable with chain `tip`. /// /// This method does not take into account the lock time. @@ -470,18 +524,20 @@ impl CanonicalView { /// [`taints_unowned`] to classify everything foreign as untrusted. /// * `is_settled` - Returns `true` for the [position](ChainPosition) of a transaction we /// consider settled (unlikely to be replaced), for example one with enough confirmations. - /// * `is_locked` - Returns `true` for a txout whose timelock is not met yet, classifying it as - /// [`Immature`](Eligibility::Immature). For an output confirmed - /// [`transitively`](ChainPosition::Confirmed::transitively), the confirmation height is only - /// an upper bound, so a relative timelock may look unmet when it is already met. + /// * `timelocks` - Returns the [`Timelocks`] of the spend path the caller will use for a txout. + /// A txout whose timelocks are not met (see + /// [`are_timelocks_met`](CanonicalTxOut::are_timelocks_met)) is + /// [`Immature`](Eligibility::Immature). Time-based locks need MTP, so on a view without it + /// (see [`CanonicalViewTask::with_mtp`]) they are never met. pub fn classify_outpoints<'a>( &'a self, outpoints: impl IntoIterator + 'a, mut does_taint: impl FnMut(&CanonicalTx>) -> bool + 'a, is_settled: impl Fn(&ChainPosition) -> bool + 'a, - is_locked: impl Fn(&CanonicalTxOut>) -> bool + 'a, + timelocks: impl Fn(&CanonicalTxOut>) -> Timelocks + 'a, ) -> impl Iterator>, Eligibility)> + 'a { let tip = self.tip.height; + let tip_mtp = self.tip_mtp; // Shared across outpoints so an ancestor reached by several of them is only walked once. let mut cache = HashMap::::new(); outpoints @@ -489,7 +545,9 @@ impl CanonicalView { .filter_map(move |op| self.txout(op)) .filter(|txo| txo.spent_by.is_none()) .map(move |txout| { - let eligibility = if !txout.is_mature(tip) || is_locked(&txout) { + let eligibility = if !txout.is_mature(tip) + || !txout.are_timelocks_met(timelocks(&txout), tip, tip_mtp) + { Eligibility::Immature } else if is_settled(&txout.pos) { Eligibility::Settled @@ -607,7 +665,7 @@ impl CanonicalView { /// This is a fold over [`classify_outpoints`](Self::classify_outpoints): each output's value is /// added to the bucket matching its [`Eligibility`]. /// - /// See `classify_outpoints` for `does_taint` and `is_settled` meaning. + /// See `classify_outpoints` for `does_taint`, `is_settled` and `timelocks` meaning. /// /// The position of an output confirmed [`transitively`](ChainPosition::Confirmed::transitively) /// carries a descendant's anchor, so its confirmation height may be an upper bound (see @@ -634,7 +692,7 @@ impl CanonicalView { /// indexer.outpoints().iter().map(|(_, op)| *op), /// bdk_chain::taints_unowned(&indexer), /// |pos: &ChainPosition<_>| pos.confirmations_lower_bound(tip_height) >= 6, - /// |_txout| false, + /// |_txout| Default::default(), /// ); /// ``` pub fn balance( @@ -642,9 +700,9 @@ impl CanonicalView { outpoints: impl IntoIterator, does_taint: impl FnMut(&CanonicalTx>) -> bool, is_settled: impl Fn(&ChainPosition) -> bool, - is_locked: impl Fn(&CanonicalTxOut>) -> bool, + timelocks: impl Fn(&CanonicalTxOut>) -> Timelocks, ) -> Balance { - self.classify_outpoints(outpoints, does_taint, is_settled, is_locked) + self.classify_outpoints(outpoints, does_taint, is_settled, timelocks) .collect() } } diff --git a/crates/chain/tests/test_canonical_view.rs b/crates/chain/tests/test_canonical_view.rs index 32455154e7..0300632cee 100644 --- a/crates/chain/tests/test_canonical_view.rs +++ b/crates/chain/tests/test_canonical_view.rs @@ -4,10 +4,13 @@ use std::collections::BTreeMap; use bdk_chain::{ local_chain::LocalChain, BlockId, CanonicalTxOut, ChainPosition, ConfirmationBlockTime, - Eligibility, Trust, TxGraph, + Eligibility, Timelocks, Trust, TxGraph, }; use bdk_testenv::{hash, utils::new_tx}; -use bitcoin::{key::Secp256k1, Amount, BlockHash, OutPoint, ScriptBuf, Transaction, TxIn, TxOut}; +use bitcoin::{ + absolute, key::Secp256k1, relative, Amount, BlockHash, OutPoint, ScriptBuf, Transaction, TxIn, + TxOut, +}; use miniscript::Descriptor; /// Builds an `is_settled` predicate requiring at least `min_confirmations` confirmations. @@ -95,7 +98,7 @@ fn test_is_settled_boundary() { [outpoint], |_tx| false, // leave trust to ancestry settled(tip_height, 1), - |_txout| false, + |_txout| Default::default(), ); assert_eq!(balance_1_conf.confirmed, Amount::from_sat(50_000)); @@ -106,7 +109,7 @@ fn test_is_settled_boundary() { [outpoint], |_tx| false, // leave trust to ancestry settled(tip_height, 6), - |_txout| false, + |_txout| Default::default(), ); assert_eq!(balance_6_conf.confirmed, Amount::from_sat(50_000)); assert_eq!(balance_6_conf.trusted_pending, Amount::ZERO); @@ -116,7 +119,7 @@ fn test_is_settled_boundary() { [outpoint], |_tx| false, // leave trust to ancestry settled(tip_height, 7), - |_txout| false, + |_txout| Default::default(), ); assert_eq!(balance_7_conf.confirmed, Amount::ZERO); assert_eq!(balance_7_conf.trusted_pending, Amount::from_sat(50_000)); @@ -199,7 +202,7 @@ fn test_min_confirmations_with_untrusted_tx() { [outpoint], |_tx| true, // taint everything settled(tip_height, 5), - |_txout| false, + |_txout| Default::default(), ); // Should be untrusted pending (not enough confirmations and not trusted) @@ -212,7 +215,7 @@ fn test_min_confirmations_with_untrusted_tx() { [outpoint], |_tx| false, // leave trust to ancestry settled(tip_height, 5), - |_txout| false, + |_txout| Default::default(), ); assert_eq!(balance.trusted_pending, Amount::from_sat(25_000)); assert_eq!(balance.untrusted_pending, Amount::ZERO); @@ -364,7 +367,7 @@ fn test_min_confirmations_multiple_transactions() { outpoints.clone(), |_tx| false, settled(tip_height, 5), - |_txout| false, + |_txout| Default::default(), ); assert_eq!( @@ -385,7 +388,7 @@ fn test_min_confirmations_multiple_transactions() { outpoints, |_tx| false, settled(tip_height, 10), - |_txout| false, + |_txout| Default::default(), ); assert_eq!( @@ -508,7 +511,7 @@ fn test_balance_taint_propagates_through_unconfirmed_ancestry() { .any(|txin| !owned.contains(&txin.previous_output)) }, |pos| pos.is_confirmed(), - |_txout| false, + |_txout| Default::default(), ); assert_eq!(balance.confirmed, Amount::ZERO); @@ -547,7 +550,12 @@ fn test_balance_is_settled_is_authoritative_for_unconfirmed() { // An `is_settled` that claims everything is settled counts the (mature, non-coinbase) // unconfirmed output as settled rather than dropping it, even when it is tainted. - let balance = view.balance([OutPoint::new(txid, 0)], |_| true, |_| true, |_txout| false); + let balance = view.balance( + [OutPoint::new(txid, 0)], + |_| true, + |_| true, + |_txout| Default::default(), + ); assert_eq!(balance.confirmed, Amount::from_sat(50_000)); assert_eq!(balance.immature, Amount::ZERO); assert_eq!(balance.trusted_pending, Amount::ZERO); @@ -628,7 +636,7 @@ fn test_balance_taint_stops_at_settled_ancestor() { .any(|txin| !owned.contains(&txin.previous_output)) }, |pos| pos.is_confirmed(), - |_txout| false, + |_txout| Default::default(), ) .map(|(txout, eligibility)| (txout.outpoint, eligibility)) .collect::>(); @@ -697,7 +705,12 @@ fn test_classify_immature_and_settled() { ]; let by_op = view - .classify_outpoints(ops, |_| false, |pos| pos.is_confirmed(), |_txout| false) + .classify_outpoints( + ops, + |_| false, + |pos| pos.is_confirmed(), + |_txout| Default::default(), + ) .map(|(txout, eligibility)| (txout.outpoint, eligibility)) .collect::>(); assert_eq!( @@ -707,7 +720,12 @@ fn test_classify_immature_and_settled() { assert_eq!(by_op[&OutPoint::new(normal_txid, 0)], Eligibility::Settled); // The balance buckets reflect the same classification. - let balance = view.balance(ops, |_| false, |pos| pos.is_confirmed(), |_txout| false); + let balance = view.balance( + ops, + |_| false, + |pos| pos.is_confirmed(), + |_txout| Default::default(), + ); assert_eq!(balance.immature, Amount::from_sat(50_000)); assert_eq!(balance.confirmed, Amount::from_sat(30_000)); } @@ -799,7 +817,7 @@ fn test_balance_taint_shared_ancestor() { .any(|txin| !owned.contains(&txin.previous_output)) }, |pos| pos.is_confirmed(), - |_txout| false, + |_txout| Default::default(), ); assert_eq!(balance.trusted_pending, Amount::ZERO); assert_eq!(balance.untrusted_pending, Amount::from_sat(29_000 + 19_000)); @@ -855,7 +873,7 @@ fn test_classify_skips_spent_and_unknown() { ], |_| false, |pos| pos.is_confirmed(), - |_txout| false, + |_txout| Default::default(), ) .collect::>(); @@ -901,7 +919,7 @@ fn test_immature_coinbase_stays_immature_when_unsettled() { [OutPoint::new(txid, 0)], |_| false, settled(tip_height, 3), - |_txout| false, + |_txout| Default::default(), ) .map(|(txout, e)| (txout.outpoint, e)) .collect::>(); @@ -947,7 +965,7 @@ fn test_mature_coinbase_is_settled_not_immature() { [OutPoint::new(txid, 0)], |_| false, settled(tip_height, 3), - |_txout| false, + |_txout| Default::default(), ) .map(|(txout, e)| (txout.outpoint, e)) .collect::>(); @@ -1003,7 +1021,7 @@ fn test_unsettled_unknown_when_parent_root_missing() { [OutPoint::new(child2_txid, 0)], |_| false, |pos| pos.is_confirmed(), - |_txout| false, + |_txout| Default::default(), ) .map(|(txout, e)| (txout.outpoint, e)) .collect::>(); @@ -1058,7 +1076,7 @@ fn test_evicted_stale_anchored_tx_not_canonical() { ); } -/// A settled output for which `is_locked` returns true is classified `Immature` and counted in +/// A settled output whose `Timelocks` are unmet is classified `Immature` and counted in /// `Balance::immature` instead of `confirmed`. #[test] fn test_classify_locked() { @@ -1092,27 +1110,36 @@ fn test_classify_locked() { let view = chain.canonicalize(&tx_graph, chain.tip().block_id(), Default::default()); let ops = [OutPoint::new(txid, 0)]; - // Timelock unmet + // Timelock unmet: the spend needs height 2, the tip is at 1. + let locked = |_: &CanonicalTxOut<_>| Timelocks { + absolute: Some(absolute::LockTime::from_consensus(2)), + relative: None, + }; let (_, eligibility) = view - .classify_outpoints(ops, |_| false, |pos| pos.is_confirmed(), |_| true) + .classify_outpoints(ops, |_| false, |pos| pos.is_confirmed(), locked) .next() .unwrap(); assert_eq!(eligibility, Eligibility::Immature); - let balance = view.balance(ops, |_| false, |pos| pos.is_confirmed(), |_| true); + let balance = view.balance(ops, |_| false, |pos| pos.is_confirmed(), locked); assert_eq!(balance.immature, Amount::from_sat(40_000)); assert_eq!(balance.confirmed, Amount::ZERO); - // Timelock met - let balance = view.balance(ops, |_| false, |pos| pos.is_confirmed(), |_| false); + // No timelock + let balance = view.balance( + ops, + |_| false, + |pos| pos.is_confirmed(), + |_| Timelocks::default(), + ); assert_eq!(balance.confirmed, Amount::from_sat(40_000)); assert_eq!(balance.immature, Amount::ZERO); } -/// `is_locked` gets what a wallet needs to evaluate its descriptors' timelocks: the spk, and the -/// position to count confirmations. Both outputs are `Immature` while unconfirmed (here also -/// untrusted). Once confirmed, the `older(100)` one is `Immature` until its 100th confirmation, -/// and the `after(200)` one until the tip reaches 200. +/// `timelocks` gets what a wallet needs to look up its descriptors' timelocks: the spk. Both +/// outputs are `Immature` while unconfirmed (here also untrusted). Once confirmed, the +/// `older(100)` one is `Immature` until its 100th confirmation, and the `after(200)` one until the +/// tip reaches 200. #[test] fn test_classify_locked_descriptor_timelocks() { let key = "tprv8ZgxMBicQKsPd3krDUsBAmtnRsK3rb8u5yi1zhQgMhF1tR8MW7xfE4rnrbbsrbPR52e7rKapu6ztw1jXveJSCGHEriUGZV7mCe88duLp5pj/0/*"; @@ -1122,10 +1149,12 @@ fn test_classify_locked_descriptor_timelocks() { desc.at_derivation_index(0).unwrap().script_pubkey() }; let (csv_spk, cltv_spk) = (spk("older(100)"), spk("after(200)")); - let is_locked = |tip: u32, txout: &CanonicalTxOut>| { + let timelocks = |txout: &CanonicalTxOut>| { let spk = &txout.txout.script_pubkey; - (*spk == csv_spk && txout.pos.confirmations_lower_bound(tip) < 100) - || (*spk == cltv_spk && tip < 200) + Timelocks { + absolute: (*spk == cltv_spk).then(|| absolute::LockTime::from_consensus(200)), + relative: (*spk == csv_spk).then(|| relative::LockTime::from_height(100)), + } }; let mut chain = @@ -1154,16 +1183,10 @@ fn test_classify_locked_descriptor_timelocks() { let classify = |chain: &LocalChain, tx_graph: &TxGraph| { let view = chain.canonicalize(tx_graph, chain.tip().block_id(), Default::default()); - let tip = view.tip().height; let ops = [OutPoint::new(txid, 0), OutPoint::new(txid, 1)]; - view.classify_outpoints( - ops, - |_| true, - |pos| pos.is_confirmed(), - |o| is_locked(tip, o), - ) - .map(|(_, eligibility)| eligibility) - .collect::>() + view.classify_outpoints(ops, |_| true, |pos| pos.is_confirmed(), timelocks) + .map(|(_, eligibility)| eligibility) + .collect::>() }; assert_eq!(classify(&chain, &tx_graph), [Eligibility::Immature; 2]); @@ -1183,6 +1206,96 @@ fn test_classify_locked_descriptor_timelocks() { } } +/// `are_timelocks_met` follows consensus for each kind of lock, one step either side of the +/// boundary, and counts a lock that needs an unknown MTP as unmet. +#[test] +fn test_are_timelocks_met_boundaries() { + // Above `LOCKTIME_THRESHOLD`, so an absolute lock this large is a time. + const MTP: u32 = 600_000_000; + let tip = 100; + + let txout = |pos, prev_mtp| CanonicalTxOut { + pos, + outpoint: OutPoint::null(), + txout: TxOut::NULL, + spent_by: None, + is_on_coinbase: false, + prev_mtp, + }; + let confirmed_at_90 = ChainPosition::Confirmed { + anchor: ConfirmationBlockTime { + block_id: BlockId { + height: 90, + hash: hash!("90"), + }, + confirmation_time: 0, + }, + transitively: None, + }; + let unconfirmed = ChainPosition::Unconfirmed { + first_seen: Some(1), + last_seen: Some(1), + }; + // Confirmed at 90, with the block before it 1024s (two intervals) older than the tip's MTP. + let confirmed = txout(confirmed_at_90, Some(MTP - 1024)); + let no_prev_mtp = txout(confirmed_at_90, None); + let mempool = txout(unconfirmed, None); + + let abs = |n| Timelocks { + absolute: Some(absolute::LockTime::from_consensus(n)), + relative: None, + }; + let blocks = |n| Timelocks { + absolute: None, + relative: Some(relative::LockTime::from_height(n)), + }; + let time = |n| Timelocks { + absolute: None, + relative: Some(relative::LockTime::from_512_second_intervals(n)), + }; + + let cases = [ + // (name, txout, timelocks, tip_mtp, expected) + ("no lock", &mempool, Timelocks::default(), None, true), + // Spendable in block 101, so a lock height of 100 is met (BIP-65). + ("abs height met", &confirmed, abs(100), None, true), + ("abs height unmet", &confirmed, abs(101), None, false), + // Met once the lock time is below the tip's MTP (BIP-113). + ("abs time met", &confirmed, abs(MTP - 1), Some(MTP), true), + ("abs time unmet", &confirmed, abs(MTP), Some(MTP), false), + ("abs time, no mtp", &confirmed, abs(MTP - 1), None, false), + // Confirmed at 90, spendable in block 101: 101 - 90 = 11 blocks (BIP-68). + ("rel blocks met", &confirmed, blocks(11), None, true), + ("rel blocks unmet", &confirmed, blocks(12), None, false), + // An unconfirmed output confirms in block 101 at the earliest. + ("rel blocks 0, mempool", &mempool, blocks(0), None, true), + ("rel blocks 1, mempool", &mempool, blocks(1), None, false), + // `prev_mtp` is 1024s (two intervals) below the tip's MTP. + ("rel time met", &confirmed, time(2), Some(MTP), true), + ("rel time unmet", &confirmed, time(3), Some(MTP), false), + ("rel time, no tip mtp", &confirmed, time(0), None, false), + ("no prev mtp", &no_prev_mtp, time(0), Some(MTP), false), + ("rel time 0, mempool", &mempool, time(0), Some(MTP), true), + ( + "both, one unmet", + &confirmed, + Timelocks { + absolute: Some(absolute::LockTime::from_consensus(100)), + relative: Some(relative::LockTime::from_height(12)), + }, + None, + false, + ), + ]; + for (name, txout, timelocks, tip_mtp, expected) in cases { + assert_eq!( + txout.are_timelocks_met(timelocks, tip, tip_mtp), + expected, + "{name}" + ); + } +} + /// An evicted transaction swept transitively as the ancestor of a stale-anchored transaction must /// still report the mempool sighting it genuinely has. /// diff --git a/crates/chain/tests/test_indexed_tx_graph.rs b/crates/chain/tests/test_indexed_tx_graph.rs index 2a46207fac..9943ba0f28 100644 --- a/crates/chain/tests/test_indexed_tx_graph.rs +++ b/crates/chain/tests/test_indexed_tx_graph.rs @@ -552,7 +552,7 @@ fn test_list_owned_txouts() { graph.index.outpoints().iter().map(|(_, op)| *op), bdk_chain::taints_unowned(&graph.index), |pos| pos.is_confirmed(), - |_txout| false, + |_txout| Default::default(), ); let confirmed_txouts_txid = txouts diff --git a/crates/chain/tests/test_tx_graph_conflicts.rs b/crates/chain/tests/test_tx_graph_conflicts.rs index 064a3aff94..da1b881175 100644 --- a/crates/chain/tests/test_tx_graph_conflicts.rs +++ b/crates/chain/tests/test_tx_graph_conflicts.rs @@ -1064,7 +1064,7 @@ fn test_tx_conflict_handling() { env.indexer.outpoints().iter().map(|(_, op)| *op), bdk_chain::taints_unowned(&env.indexer), |pos| pos.is_confirmed(), - |_txout| false, + |_txout| Default::default(), ); assert_eq!( balance, scenario.exp_balance, diff --git a/crates/electrum/tests/test_electrum.rs b/crates/electrum/tests/test_electrum.rs index d25587e4ab..6caf9af8a4 100644 --- a/crates/electrum/tests/test_electrum.rs +++ b/crates/electrum/tests/test_electrum.rs @@ -66,7 +66,7 @@ fn get_balance( outpoints.into_iter().map(|(_, op)| op), bdk_chain::taints_unowned(&recv_graph.index), |pos| pos.is_confirmed(), - |_txout| false, + |_txout| Default::default(), ); Ok(balance) }