The `TxGraph.list_chain_txs()` method doesn't behave as you'd expect from the docs

When calling the TxGraph.list_chain_txs(chain, chain_tip) method, if you pass a chain_tip that does not belong in the chain, the method will simply return all transactions in your transaction graph, and transactions wil appear to be unconfirmed rather than the method letting your know that your chain_tip and chain are not currently compatible. The API docs for the method read:

List graph transactions that are in chain with chain_tip.

Consider the following example:

fn main() -> () {
    pub const RAW_TX_1: &str = "0200000000010116d6174da7183d70d0a7d4dc314d517a7d135db79ad63515028b293a76f4f9d10000000000feffffff023a21fc8350060000160014531c405e1881ef192294b8813631e258bf98ea7a1027000000000000225120a60869f0dbcf1dc659c9cecbaf8050135ea9e8cdc487053f1dc6880949dc684c024730440220591b1a172a122da49ba79a3e79f98aaa03fd7a372f9760da18890b6a327e6010022013e82319231da6c99abf8123d7c07e13cf9bd8d76e113e18dc452e5024db156d012102318a2d558b2936c52e320decd6d92a88d7f530be91b6fe0af5caf41661e77da3ef2e0100";
    let tx: Transaction = tx_from_hex(RAW_TX_1);

    let mut graph = TxGraph::<ConfirmationTimeHeightAnchor>::default();
    graph.insert_tx(tx.clone());

    let confirmation_time_height_anchor2 = ConfirmationTimeHeightAnchor {
        anchor_block: BlockId {
            height: 2,
            hash: Hash::hash("second".as_bytes()),
        },
        confirmation_height: 2,
        confirmation_time: 100,
    };

    let chain = LocalChain::from_blocks(
        [
            (0, Hash::hash("zero".as_bytes())),
            (1, Hash::hash("first".as_bytes())),
            (2, Hash::hash("second".as_bytes())),
            (3, Hash::hash("third".as_bytes())),
        ]
            .into_iter()
            .collect::<BTreeMap<u32, BlockHash>>(),
    ).unwrap();

    graph.insert_anchor(
        tx.txid(),
        confirmation_time_height_anchor2
    );

    let block_4 = BlockId {
        height: 4,
        hash: Hash::hash("fourth".as_bytes()),
    };

    let txs = graph.list_chain_txs(&chain, block_4);
    println!("Transactions: {:#?}\n", txs.collect::<Vec<_>>());
}

This example will print to the console

Transactions: [
    CanonicalTx {
        chain_position: Unconfirmed(
            0,
        ),
        tx_node: TxNode {
            txid: 0xd6f6c49aadcc8f5a3d5997b8e564acf443af0dd7d6bb2e5b34d68d9ea90c9838,
            tx: Transaction {
                version: 2,
                lock_time: Blocks(
                    Height(
                        77551,
                    ),
                ),
                input: ...

instead of warning the user that they provided a chain tip that does not exist in the chain.

I opened this issue to ask whether this is intended behaviour (and we can modify the docs to reflect that) or whether this might be a small bug.

This is correct behavior. The docs are bad.

My proposal:

/// List all transactions with their respective positions in `chain` with `chain_tip`.

Ok good to know this is intended behaviour! Now I can start digging into why it's the case. I'm not sure if your proposed docs make it clear just yet (at least for me).

For example, note that my transaction in the code above was anchored to block 2, which is actually in the chain I'm passing to the method (blocks 0, 1, 2, 3). The only problem with it is that the chain tip is not in this chain. Given the doc List all transactions with their respective positions in chain, I would have expected the transaction to show up as confirmed (or for the method to tell me that there is a problem with my chain somehow). The idea that because I provided an incompatible tip my transaction is now unconfirmed (even though it's anchored to a block that's valid and in my chain) is not intuitive to me.

Maybe I don't have the correct mental model for this? If so I need to build one (let me know if you see where I'm getting lost). Once I understand why this is intended/preferred behaviour, I can comment on the docstring and/or propose improvements.

chain is the Chain Oracle implementation. chain_tip identifies a chain of blocks (because of the nature of the Blockchain being immutable). We need a chain_tip parameter to ensure multiple calls to the Chain Oracle will be consistent. I.e. positions of transactions returned will not be in conflicting blocks.

When we use LocalChain as the Chain Oracle, the unfortunate fact is that LocalChain does not know everything, and does not know whether some blocks belong to a given tip (i.e. the example you just gave).

Anything that can't be known to be anchored to a block under the chain identified be chain_tip is assumed to be unconfirmed. This is usually the behavior you want and you'll just need to update the LocalChain or insert additional Anchors to eventually have the logic figure it out.

Ok I think I understand better now. Here is my new mental model for it, let me know if I'm going wrong somewhere.

My mistake was thinking that the chain argument was somehow the most "important" one (whatever that means but you get my drift), when in reality the chain_tip is the big boss of them two. We assume the chain_tip to be truth (the tip implies all blocks behind it of course), and the chain: LocalChain argument either "connects" to this tip or it doesn't. If it doesn't, then you can't really trust anything in this chain, and simply mark everything as unconfirmed (whether the txs are anchored in that chain or not does not matter, since the chain is potentially wrong/reorged/whatever, and will not/cannot really be trusted until it can be connected to the tip provided).

Here is an updated code example showcasing this behaviour (note the 3 added lines at the bottom), which shows the transaction anchored in block 2 to be confirmed on the second call now that the localchain is connected to the chain tip argument passed to the method:

fn main() -> () {
    pub const RAW_TX_1: &str = "0200000000010116d6174da7183d70d0a7d4dc314d517a7d135db79ad63515028b293a76f4f9d10000000000feffffff023a21fc8350060000160014531c405e1881ef192294b8813631e258bf98ea7a1027000000000000225120a60869f0dbcf1dc659c9cecbaf8050135ea9e8cdc487053f1dc6880949dc684c024730440220591b1a172a122da49ba79a3e79f98aaa03fd7a372f9760da18890b6a327e6010022013e82319231da6c99abf8123d7c07e13cf9bd8d76e113e18dc452e5024db156d012102318a2d558b2936c52e320decd6d92a88d7f530be91b6fe0af5caf41661e77da3ef2e0100";
    let tx: Transaction = tx_from_hex(RAW_TX_1);

    let mut graph = TxGraph::<ConfirmationTimeHeightAnchor>::default();
    graph.insert_tx(tx.clone());

    let confirmation_time_height_anchor2 = ConfirmationTimeHeightAnchor {
        anchor_block: BlockId {
            height: 2,
            hash: Hash::hash("second".as_bytes()),
        },
        confirmation_height: 2,
        confirmation_time: 100,
    };

    let mut chain = LocalChain::from_blocks(
        [
            (0, Hash::hash("zero".as_bytes())),
            (1, Hash::hash("first".as_bytes())),
            (2, Hash::hash("second".as_bytes())),
            (3, Hash::hash("third".as_bytes())),
        ]
            .into_iter()
            .collect::<BTreeMap<u32, BlockHash>>(),
    ).unwrap();

    graph.insert_anchor(
        tx.txid(),
        confirmation_time_height_anchor2
    );

    let block_42 = BlockId {
        height: 42,
        hash: Hash::hash("fourtytwo".as_bytes()),
    };

    let txs1 = graph.list_chain_txs(&chain, block_42);
    println!("Transactions in localchain [0, 1, 2, 3] but tip 42: \n{:#?}\n", txs1.collect::<Vec<_>>());

    chain.insert_block(block_42);
    let txs2 = graph.list_chain_txs(&chain, block_42);
    println!("Transactions in localchain [0, 1, 2, 3, 42] and tip 42: \n{:#?}\n", txs2.collect::<Vec<_>>());
}

Thank you for helping me through this @evanlinjin. I think my I can now better comment on the shape of the API: in this situation, the fact that the chain_tip and the chain do not in fact connect is a fairly big deal (it completely invalidates your LocalChain and will treat everything as unconfirmed). I think this could be communicated maybe better to the users through the docs. What do you think of the following:

/// List all transactions with their respective positions in `chain` with `chain_tip`.
///
/// Note that if you provide a `chain` that does not contain `chain_tip`, all transactions in your
/// `TxGraph` that are in this chain will be considered unconfirmed (since they will not be
/// anchored to blocks considered part of the chain represented by `chain_tip`).

How is list_chain_txs returning things as last_seen as 0 when @thunderbiscuit didn't even insert a last seen for that tx though.

I updated the example code to run on v1.0.0-beta.2 and found that passing in a bogus chain tip does not result in the graph blindly returning all of its transactions regardless of anchor status. I suspect this was corrected around the time we changed list_chain_txs to list_canonical_transactions. @thunderbiscuit Let me know if something is still unclear

use std::collections::BTreeMap;

use bdk_wallet::bitcoin::{consensus, hashes::Hash, hex::FromHex, BlockHash, Transaction};
use bdk_wallet::chain::{local_chain::LocalChain, BlockId, ConfirmationBlockTime, TxGraph};

fn main() -> anyhow::Result<()> {
    pub const RAW_TX_1: &str = "0200000000010116d6174da7183d70d0a7d4dc314d517a7d135db79ad63515028b293a76f4f9d10000000000feffffff023a21fc8350060000160014531c405e1881ef192294b8813631e258bf98ea7a1027000000000000225120a60869f0dbcf1dc659c9cecbaf8050135ea9e8cdc487053f1dc6880949dc684c024730440220591b1a172a122da49ba79a3e79f98aaa03fd7a372f9760da18890b6a327e6010022013e82319231da6c99abf8123d7c07e13cf9bd8d76e113e18dc452e5024db156d012102318a2d558b2936c52e320decd6d92a88d7f530be91b6fe0af5caf41661e77da3ef2e0100";
    let data = <Vec<u8> as FromHex>::from_hex(RAW_TX_1)?;
    let tx: Transaction = consensus::deserialize(&data)?;

    let mut graph = TxGraph::<ConfirmationBlockTime>::default();
    let _ = graph.insert_tx(tx.clone());
    let confirmation_time_height_anchor2 = ConfirmationBlockTime {
        block_id: BlockId {
            height: 2,
            hash: Hash::hash("second".as_bytes()),
        },
        confirmation_time: 100,
    };
    let _ = graph.insert_anchor(tx.compute_txid(), confirmation_time_height_anchor2);

    let chain = LocalChain::from_blocks(
        [
            (0, Hash::hash("zero".as_bytes())),
            (1, Hash::hash("first".as_bytes())),
            (2, Hash::hash("second".as_bytes())),
            (3, Hash::hash("third".as_bytes())),
        ]
        .into_iter()
        .collect::<BTreeMap<u32, BlockHash>>(),
    )?;

    let chain_tip = chain.tip().block_id();
    assert_eq!(chain_tip.height, 3);
    let txs = graph.list_canonical_txs(&chain, chain_tip);
    assert_eq!(txs.count(), 1);

    let block_4 = BlockId {
        height: 4,
        hash: Hash::hash("fourth".as_bytes()),
    };
    let mut txs = graph.list_canonical_txs(&chain, block_4);
    assert!(txs.next().is_none());

    Ok(())
}

bitcoindevkit / bdk

The `TxGraph.list_chain_txs()` method doesn't behave as you'd expect from the docs #1297