Skip to content

CanonicalView admits an unconfirmed coinbase transaction #2269

Description

@evanlinjin

Describe the bug

CanonicalView can hold a coinbase transaction at ChainPosition::Unconfirmed, which is not a state that can exist on any real chain. A coinbase is only ever created by a block, so it is confirmed or it does not exist.

The CanonicalStage::AssumedTxs stage (canonical_task.rs:88) has no coinbase check. When a caller passes a coinbase txid via CanonicalParams::assume_canonical and that tx has no direct anchor and no anchored descendant, canonical_view_task.rs:183 resolves it to ChainPosition::Unconfirmed.

Note the asymmetry: CanonicalStage::SeenTxs already guards against this, but only with a debug_assert!:

debug_assert!(
    !tx.is_coinbase(),
    "Coinbase txs must not have `last_seen` (in mempool) value"
);

In release builds that assert is compiled out and the coinbase flows through to mark_canonical with ObservedIn::Mempool, reaching the same unconfirmed position. So both stages can produce the invalid state; AssumedTxs does it in debug builds too.

Downstream code reasonably assumes the invariant holds. CanonicalTxOut::is_mature has:

None => {
    debug_assert!(false, "coinbase tx can never be unconfirmed");
    return false;
}

so any consumer that calls is_mature on such an output panics in debug builds and silently gets false in release.

To Reproduce

#[test]
fn assumed_canonical_coinbase_is_unconfirmed() {
    let coinbase = Transaction {
        version: transaction::Version::ONE,
        lock_time: absolute::LockTime::ZERO,
        input: vec![TxIn {
            previous_output: OutPoint::null(),
            script_sig: ScriptBuf::new(),
            sequence: Sequence::MAX,
            witness: Witness::new(),
        }],
        output: vec![TxOut {
            value: Amount::from_sat(50_000),
            script_pubkey: ScriptBuf::new(),
        }],
    };
    assert!(coinbase.is_coinbase());
    let txid = coinbase.compute_txid();

    let mut graph = TxGraph::<BlockId>::default();
    let _ = graph.insert_tx(coinbase); // no anchor, no last_seen
    let chain =
        LocalChain::from_blocks([(0, BlockHash::all_zeros())].into_iter().collect()).unwrap();
    let tip = chain.tip().block_id();

    let view = chain.canonical_view(
        &graph,
        tip,
        CanonicalParams { assume_canonical: vec![txid] },
    );

    let ctx = view.tx(txid).expect("coinbase is canonical");
    assert!(matches!(ctx.pos, ChainPosition::Unconfirmed { .. })); // passes
}

On master this assertion passes, printing position = Unconfirmed { first_seen: None, last_seen: None }.

Expected behavior

A coinbase transaction should never end up in a CanonicalView at an unconfirmed position. A coinbase with no anchor and no anchored descendant has no claim to being canonical, so it should be excluded during canonicalization rather than admitted at an impossible position.

The guard belongs in CanonicalTask where all stages route through, so AssumedTxs, SeenTxs and any future stage are covered at once, and it should hold in release builds — not just as a debug_assert!.

Build environment

  • BDK tag/commit: master @ acc06e5
  • Crate: bdk_chain

Additional context

Found while reviewing #2246 (classify_outpoints). That branch makes the latent bug louder rather than causing it: the new classify_outpoints calls is_mature() before any position check, so the debug_assert!(false, "coinbase tx can never be unconfirmed") becomes reachable through balance(). The underlying invalid state predates that work and reproduces on master as shown above.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    • Status
      No status

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions