From: 志宇 Date: Thu, 10 Sep 2026 10:58:30 +0000 (+0000) Subject: docs(core,chain): tighten the `ChainTask` driver contract X-Git-Tag: pre-fold-backup~1 X-Git-Url: http://internal-gitweb-vhost/-script/src/example_cli/database/encode/struct.EncoderStringWriter.html?a=commitdiff_plain;h=6f73dc981ab7ebffee33b548857bf96f33ed455c;p=bdk docs(core,chain): tighten the `ChainTask` driver contract Two gaps in the protocol between a task and its driver, one on each side. 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. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NND8BhEmYTYdLRv4dbUUro --- diff --git a/crates/chain/src/local_chain.rs b/crates/chain/src/local_chain.rs index 0ff942ff..f9621af7 100644 --- a/crates/chain/src/local_chain.rs +++ b/crates/chain/src/local_chain.rs @@ -419,6 +419,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: Q) -> Q::Output where Q: ChainTask, @@ -441,11 +448,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_local_chain.rs b/crates/chain/tests/test_local_chain.rs index bc5151d9..7affe38e 100644 --- a/crates/chain/tests/test_local_chain.rs +++ b/crates/chain/tests/test_local_chain.rs @@ -1311,3 +1311,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 b6eb0416..48d1a694 100644 --- a/crates/core/src/block_queries.rs +++ b/crates/core/src/block_queries.rs @@ -41,6 +41,11 @@ 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". pub fn resolve(&mut self, height: u32, block: Option) { let was_pending = self.pending.remove(&height); self.blocks.insert(height, block); diff --git a/crates/core/src/chain_task.rs b/crates/core/src/chain_task.rs index 8cd7f6be..d1109a4e 100644 --- a/crates/core/src/chain_task.rs +++ b/crates/core/src/chain_task.rs @@ -117,14 +117,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.