Have MTP in checkpoint data and report missing blocks

[evanlinjin]

Description

Context: #2076 (comment)

Carries median-time-past (MTP) on each checkpoint at the type level, and reworks compute_mtp so callers can drive fetch-retry loops when ancestor blocks are missing.

• WithMtp<D> wrapper: CheckPoint<WithMtp<D>> is a compile-time guarantee that MTP is present on every checkpoint in the chain. Code that needs MTP can take CheckPoint<WithMtp<D>> and read it via CheckPoint::mtp() without ever having to handle a "MTP couldn't be computed" case — the type system rules that out. Compare to the alternative of computing MTP on demand, which forces every consumer to deal with the failure path even when the chain source already has the data.
• compute_mtp returns Result<u32, MissingBlocks> instead of Option<u32>. MissingBlocks lists every height in the MTP window (h-10..=h) that the chain is missing, so callers — especially light-client chain sources like Electrum that don't expose MTP directly — can batch-fetch exactly the headers they need and retry.
• Rename median_time_past → compute_mtp to distinguish the chain-walking constructor from the mtp() accessor on CheckPoint<WithMtp<D>>.

Notes to the reviewers

Breaking change, marked feat(core)!. Two API breaks:

• median_time_past → compute_mtp
• compute_mtp returns Result<u32, MissingBlocks> (was Option<u32>)

Changelog notice

Added:
- `WithMtp<D>` wrapper: `CheckPoint<WithMtp<D>>` guarantees at the type level that MTP is present for every checkpoint.
- `CheckPoint::mtp()` on `CheckPoint<WithMtp<D>>`.
- `MissingBlocks` error type.

Changed:
- `CheckPoint::median_time_past` renamed to `CheckPoint::compute_mtp`; return type changed from `Option<u32>` to `Result<u32, MissingBlocks>`.

Checklists

All Submissions:

• I followed the contribution guidelines

New Features:

• I've added tests for the new feature
• I've added docs for the new feature

[evanlinjin]

I'm wondering whether we should get rid of the ToBlockTime trait and implement block-time-dependent methods directly on CheckPoint<Header>. This approach is more aligned with the YAGNI principle. I'm personally not expecting many custom checkpoint data types. WithMtp<Header> seems like what the super majority of wallets will use.

The other approach is to go the other way and introduce a ToBlockMTP trait to this PR.

[notmandatory]

This generally looks like a reasonable performance optimization. Would blockchain clients be responsible for recomputing the recent mpc values in the case of a reorg?

[notmandatory]

Is it worth mentioning here that the MTP value returned here would be in accurate in the case of a reorg that happens after the MTP was calculated?

[notmandatory]

I'm wondering whether we should get rid of the ToBlockTime trait and implement block-time-dependent methods directly on CheckPoint<Header>. This approach is more aligned with the YAGNI principle. I'm personally not expecting many custom checkpoint data types. WithMtp<Header> seems like what the super majority of wallets will use.

The other approach is to go the other way and introduce a ToBlockMTP trait to this PR.

Wouldn't any custom CheckPoint<D> type need to implement D: ToBlockTime anyway? so probably would be better to implement these directly on CheckPoint rather than this wrapper approach.

[evanlinjin]

@notmandatory Thanks for the review!

This generally looks like a reasonable performance optimization. Would blockchain clients be responsible for recomputing the recent mpc values in the case of a reorg?

The main intention behind the PR is not about performance (the previous PR description was misleading and I've updated it!). The idea is to enforce MTP value per checkpoint at compile time - if you use CheckPoint<WithMtp<Header>>, you have access to the method .mtp() that returns the MTP value (not wrapped in Option). If you have CheckPoint<Header>, you calculate_mtp() that returns an option (as the MTP time needs to be calculated from the previous 11 headers).

So the chain source must return the MTP value per block in order to be compatible with WithMtp<X> types. Bitcoind's RPC methods already do this. Electrum does not (as you are expected to have most of the headers and verify PoW). For bdk_electrum, we can artificially allow grabbing the MTP value by querying 11 blocks.

Wouldn't any custom CheckPoint<D> type need to implement D: ToBlockTime anyway? so probably would be better to implement these directly on CheckPoint rather than this wrapper approach.

So currently, yes if they want the calculate_mtp() method to be available. Sorry, can you expand on this point please?