Drop the "anchor may be a descendant" framing; introduce `Anchor::confirmation_height`

Aperta
#2,298 2 commenti 0 reazioni 1 assegnatario Vedi su GitHub

@evanlinjin ci sta già lavorando.

Dal 17/9/2026.

Valutazione

Difficoltà
4/5
Tempo stimato
3-5 giorni
Idoneità per principianti
52/100
Tipo di issue
Refactoring
Chiarezza
Abbastanza chiara
Stato di attività
Attiva
Stack tecnologico
rust
Ambito
blockchain

Direzione di ricerca

Inizia con il Anchor trait in crates/chain/src/tx_data_traits.rs e ChainPosition in crates/chain/src/chain_data.rs, quindi segui i punti di chiamata elencati in canonical.rs, canonical_task.rs, canonical_view_task.rs, tx_graph.rs e tx_data_traits.rs. Aggiorna la documentazione pubblica e i compatibility shim, rinomina le chiamate interne e conferma che le implementazioni anchor già presenti nell’albero e il loro comportamento rimangano invariati.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

Summary

The Anchor trait documents that an anchor block may be a descendant of the block that actually confirmed a transaction. Nothing in the codebase produces such an anchor, and no chain source can. The framing costs us an "upper bound" name and a trail of pessimistic caveats through the public API for a case that does not occur.

Proposal: drop the framing, and introduce Anchor::confirmation_height alongside a deprecated confirmation_height_upper_bound.

The framing

crates/chain/src/tx_data_traits.rs:

If transaction A is anchored in block B, and block B is in the best chain, we can assume that transaction A is also confirmed in the best chain. This does not necessarily mean that transaction A is confirmed in block B. It could also mean transaction A is confirmed in a parent block of B.

and on the method:

Get the upper bound of the chain data's confirmation height.
The default definition gives a pessimistic answer.

Why it does not hold in practice

Every anchor constructed in this repo uses the confirming block itself:

  • electrum (crates/electrum/src/bdk_electrum_client.rs:600) builds the anchor only after validate_merkle_proof(&txid, &header.merkle_root, &proof) succeeds — the block provably contains the transaction.
  • esplora (crates/esplora/src/lib.rs:54, and the *_ext.rs sites) uses TxStatus { block_height, block_hash }, which is the confirming block.
  • TxPosInBlock (crates/chain/src/tx_data_traits.rs:111) carries tx_pos, the transaction's index within that block — structurally the confirming block. Both From<TxPosInBlock> impls (BlockId, ConfirmationBlockTime) pass block_id straight through.

Both in-tree Anchor impls define confirmation_height_upper_bound() as exactly anchor_block().height.

What it costs

The pessimism propagates into unrelated public docs. CanonicalTxOut::is_mature and is_confirmed_and_spendable (crates/chain/src/canonical.rs:148, :175) both carry:

Depending on the implementation of confirmation_height_upper_bound in Anchor, this method may return false-negatives. In other words, interpreted confirmation count may be less than the actual value.

So a caller reading the maturity API is told their coin may be reported immature when it isn't — a caveat that is unreachable with any anchor type that exists.

It also leaves ambiguity in ordering decisions. BlockQueries::resolve_candidates picks the lowest in-chain candidate among a transaction's anchors; under the current framing "several anchors in the best chain at once" is legal, so that choice is load-bearing. If anchors always name the confirming block, at most one can be in the best chain and the tie-break is defensive only.

Proposed change

1. Remove the "parent block" paragraph from the Anchor trait docs, and the "pessimistic answer" note from the method. State instead that the anchor block is the block that confirmed the transaction.

2. Add confirmation_height, deprecate confirmation_height_upper_bound.

The delegation direction matters. The obvious version is wrong:

// WRONG: silently ignores existing overrides
fn confirmation_height(&self) -> u32 { self.anchor_block().height }

#[deprecated(note = "use `confirmation_height`")]
fn confirmation_height_upper_bound(&self) -> u32 { self.confirmation_height() }

A downstream impl that overrides confirmation_height_upper_bound today keeps compiling, but every internal call site moves to confirmation_height and stops seeing that override — a silent behaviour change, the worst outcome for a deprecation.

Default the new method to the old one instead, so existing overrides continue to be honoured:

fn confirmation_height(&self) -> u32 {
    #[allow(deprecated)]
    self.confirmation_height_upper_bound()
}

#[deprecated(since = "TBD", note = "use `confirmation_height`")]
fn confirmation_height_upper_bound(&self) -> u32 {
    self.anchor_block().height
}

Then drop confirmation_height_upper_bound in a later release, at which point confirmation_height takes the anchor_block().height default.

3. Rename ChainPosition::confirmation_height_upper_bound (crates/chain/src/chain_data.rs:78) to match. This is an inherent method on a public type, so it needs its own #[deprecated] shim.

4. Update the caveats in canonical.rs:148 and :175 that only exist to describe the loose bound.

Scope

Roughly 12 call sites across canonical.rs, canonical_task.rs, canonical_view_task.rs, chain_data.rs, tx_graph.rs (insert_anchor's txs_by_highest_conf_heights bookkeeping), and tx_data_traits.rs. No behaviour change — both in-tree impls already return anchor_block().height.

Open question

Is the "parent block of B" case something a chain source was ever expected to use — a compact-block or filter-based source that learns "confirmed at or before height N" without learning the exact block? If a design has that in mind, this should stay and the docs should name that source instead of leaving it hypothetical.

Lingua principale
Rust
Stelle
1.1k
Fork
491
Merge medio
1g 5h
PR unite (30g)
1

Guida per i contributori

Apri la guida per i contributori

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di bitcoindevkit/bdk

Tutte le issue di bitcoindevkit/bdk

Issue simili

Altre issue su Rust

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.