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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NND8BhEmYTYdLRv4dbUUro
/// against this chain and returning the task's output. The chain responds with its data
/// type `D`, so `LocalChain<Header>` responds with headers and `LocalChain<BlockHash>`
/// 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<Q>(&self, mut task: Q) -> Q::Output
where
Q: ChainTask<D>,
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"
)
}
}
.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<BlockHash> 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<BlockHash>) {}
+
+ fn unresolved_queries<'a>(&'a self) -> impl Iterator<Item = u32> + '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(),
+ });
+}
}
/// 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<B>) {
let was_pending = self.pending.remove(&height);
self.blocks.insert(height, block);
/// 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<B>);
/// 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<Item = u32> + 'a;
/// Consumes the task and returns the final output.
projects / bdk / commitdiff