]> Untitled Git - bdk/commitdiff
[chain_redesign] `BlockId` should not implement `Anchor`
author志宇 <hello@evanlinjin.me>
Wed, 3 May 2023 08:03:23 +0000 (16:03 +0800)
committer志宇 <hello@evanlinjin.me>
Fri, 5 May 2023 08:38:29 +0000 (16:38 +0800)
If `BlockId` implements `Anchor`, the meaning is ambiguous. We cannot
tell whether it means the tx is anchors at the block, or whether it also
means the tx is confirmed at that block.

Instead, `ConfirmationHeightAnchor` and `ConfirmationTimeAnchor` structs
are introduced as non-ambiguous `Anchor` implementations.

Additionally, `TxGraph::relevant_heights` is removed because it is also
ambiguous. What heights are deemed relevant? A simpler and more flexible
method `TxGraph::all_anchors` is introduced instead.

crates/chain/src/chain_data.rs
crates/chain/src/tx_graph.rs
crates/chain/tests/test_indexed_tx_graph.rs
crates/chain/tests/test_tx_graph.rs

index a360b304e093a75076e9b917a81188a4ec0468e9..3252febc2563d39f18464779f2622b5e0cc2dd03 100644 (file)
@@ -160,12 +160,6 @@ impl Default for BlockId {
     }
 }
 
-impl Anchor for BlockId {
-    fn anchor_block(&self) -> BlockId {
-        *self
-    }
-}
-
 impl From<(u32, BlockHash)> for BlockId {
     fn from((height, hash): (u32, BlockHash)) -> Self {
         Self { height, hash }
@@ -187,6 +181,58 @@ impl From<(&u32, &BlockHash)> for BlockId {
     }
 }
 
+/// An [`Anchor`] implementation that also records the exact confirmation height of the transaction.
+#[derive(Debug, Default, Clone, PartialEq, Eq, Copy, PartialOrd, Ord, core::hash::Hash)]
+#[cfg_attr(
+    feature = "serde",
+    derive(serde::Deserialize, serde::Serialize),
+    serde(crate = "serde_crate")
+)]
+pub struct ConfirmationHeightAnchor {
+    /// The anchor block.
+    pub anchor_block: BlockId,
+
+    /// The exact confirmation height of the transaction.
+    ///
+    /// It is assumed that this value is never larger than the height of the anchor block.
+    pub confirmation_height: u32,
+}
+
+impl Anchor for ConfirmationHeightAnchor {
+    fn anchor_block(&self) -> BlockId {
+        self.anchor_block
+    }
+
+    fn confirmation_height_upper_bound(&self) -> u32 {
+        self.confirmation_height
+    }
+}
+
+/// An [`Anchor`] implementation that also records the exact confirmation time and height of the
+/// transaction.
+#[derive(Debug, Default, Clone, PartialEq, Eq, Copy, PartialOrd, Ord, core::hash::Hash)]
+#[cfg_attr(
+    feature = "serde",
+    derive(serde::Deserialize, serde::Serialize),
+    serde(crate = "serde_crate")
+)]
+pub struct ConfirmationTimeAnchor {
+    /// The anchor block.
+    pub anchor_block: BlockId,
+
+    pub confirmation_height: u32,
+    pub confirmation_time: u64,
+}
+
+impl Anchor for ConfirmationTimeAnchor {
+    fn anchor_block(&self) -> BlockId {
+        self.anchor_block
+    }
+
+    fn confirmation_height_upper_bound(&self) -> u32 {
+        self.confirmation_height
+    }
+}
 /// A `TxOut` with as much data as we can retrieve about it
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
 pub struct FullTxOut<P> {
index d8b13030305e2b8a138a23552da4110720eddeac..e75255e4af4fc7dbe1ee998f121183f9b3415cca 100644 (file)
@@ -349,6 +349,11 @@ impl<A> TxGraph<A> {
             .filter(move |(_, conflicting_txid)| *conflicting_txid != txid)
     }
 
+    /// Get all transaction anchors known by [`TxGraph`].
+    pub fn all_anchors(&self) -> &BTreeSet<(A, Txid)> {
+        &self.anchors
+    }
+
     /// Whether the graph has any transactions or outputs in it.
     pub fn is_empty(&self) -> bool {
         self.txs.is_empty()
@@ -592,21 +597,6 @@ impl<A: Clone + Ord> TxGraph<A> {
 }
 
 impl<A: Anchor> TxGraph<A> {
-    /// Get all heights that are relevant to the graph.
-    pub fn relevant_heights(&self) -> impl Iterator<Item = u32> + '_ {
-        let mut last_height = Option::<u32>::None;
-        self.anchors
-            .iter()
-            .map(|(a, _)| a.anchor_block().height)
-            .filter(move |&height| {
-                let is_unique = Some(height) != last_height;
-                if is_unique {
-                    last_height = Some(height);
-                }
-                is_unique
-            })
-    }
-
     /// Get the position of the transaction in `chain` with tip `chain_tip`.
     ///
     /// If the given transaction of `txid` does not exist in the chain of `chain_tip`, `None` is
index 78f3c3a6490875b687992495037833e002ced9f2..d88c1e3983f0b3a91c6da34cce216b24bd5d2bc4 100644 (file)
@@ -8,7 +8,7 @@ use bdk_chain::{
     keychain::{Balance, DerivationAdditions, KeychainTxOutIndex},
     local_chain::LocalChain,
     tx_graph::Additions,
-    BlockId, ObservedAs,
+    ConfirmationHeightAnchor, ObservedAs,
 };
 use bitcoin::{secp256k1::Secp256k1, BlockHash, OutPoint, Script, Transaction, TxIn, TxOut};
 use miniscript::Descriptor;
@@ -28,7 +28,7 @@ fn insert_relevant_txs() {
     let spk_0 = descriptor.at_derivation_index(0).script_pubkey();
     let spk_1 = descriptor.at_derivation_index(9).script_pubkey();
 
-    let mut graph = IndexedTxGraph::<BlockId, KeychainTxOutIndex<()>>::default();
+    let mut graph = IndexedTxGraph::<ConfirmationHeightAnchor, KeychainTxOutIndex<()>>::default();
     graph.index.add_keychain((), descriptor);
     graph.index.set_lookahead(&(), 10);
 
@@ -118,7 +118,8 @@ fn test_list_owned_txouts() {
     let (desc_1, _) = Descriptor::parse_descriptor(&Secp256k1::signing_only(), "tr(tprv8ZgxMBicQKsPd3krDUsBAmtnRsK3rb8u5yi1zhQgMhF1tR8MW7xfE4rnrbbsrbPR52e7rKapu6ztw1jXveJSCGHEriUGZV7mCe88duLp5pj/86'/1'/0'/0/*)").unwrap();
     let (desc_2, _) = Descriptor::parse_descriptor(&Secp256k1::signing_only(), "tr(tprv8ZgxMBicQKsPd3krDUsBAmtnRsK3rb8u5yi1zhQgMhF1tR8MW7xfE4rnrbbsrbPR52e7rKapu6ztw1jXveJSCGHEriUGZV7mCe88duLp5pj/86'/1'/0'/1/*)").unwrap();
 
-    let mut graph = IndexedTxGraph::<BlockId, KeychainTxOutIndex<String>>::default();
+    let mut graph =
+        IndexedTxGraph::<ConfirmationHeightAnchor, KeychainTxOutIndex<String>>::default();
 
     graph.index.add_keychain("keychain_1".into(), desc_1);
     graph.index.add_keychain("keychain_2".into(), desc_2);
@@ -206,86 +207,94 @@ fn test_list_owned_txouts() {
     // For unconfirmed txs we pass in `None`.
 
     let _ = graph.insert_relevant_txs(
-        [&tx1, &tx2, &tx3, &tx6]
-            .iter()
-            .enumerate()
-            .map(|(i, tx)| (*tx, [local_chain.get_block(i as u32).unwrap()])),
+        [&tx1, &tx2, &tx3, &tx6].iter().enumerate().map(|(i, tx)| {
+            (
+                *tx,
+                local_chain
+                    .get_block(i as u32)
+                    .map(|anchor_block| ConfirmationHeightAnchor {
+                        anchor_block,
+                        confirmation_height: anchor_block.height,
+                    }),
+            )
+        }),
         None,
     );
 
     let _ = graph.insert_relevant_txs([&tx4, &tx5].iter().map(|tx| (*tx, None)), Some(100));
 
     // A helper lambda to extract and filter data from the graph.
-    let fetch = |ht: u32, graph: &IndexedTxGraph<BlockId, KeychainTxOutIndex<String>>| {
-        let txouts = graph
-            .list_owned_txouts(&local_chain, local_chain.get_block(ht).unwrap())
-            .collect::<Vec<_>>();
-
-        let utxos = graph
-            .list_owned_unspents(&local_chain, local_chain.get_block(ht).unwrap())
-            .collect::<Vec<_>>();
-
-        let balance = graph.balance(
-            &local_chain,
-            local_chain.get_block(ht).unwrap(),
-            |spk: &Script| trusted_spks.contains(spk),
-        );
-
-        assert_eq!(txouts.len(), 5);
-        assert_eq!(utxos.len(), 4);
-
-        let confirmed_txouts_txid = txouts
-            .iter()
-            .filter_map(|full_txout| {
-                if matches!(full_txout.chain_position, ObservedAs::Confirmed(_)) {
-                    Some(full_txout.outpoint.txid)
-                } else {
-                    None
-                }
-            })
-            .collect::<BTreeSet<_>>();
-
-        let unconfirmed_txouts_txid = txouts
-            .iter()
-            .filter_map(|full_txout| {
-                if matches!(full_txout.chain_position, ObservedAs::Unconfirmed(_)) {
-                    Some(full_txout.outpoint.txid)
-                } else {
-                    None
-                }
-            })
-            .collect::<BTreeSet<_>>();
-
-        let confirmed_utxos_txid = utxos
-            .iter()
-            .filter_map(|full_txout| {
-                if matches!(full_txout.chain_position, ObservedAs::Confirmed(_)) {
-                    Some(full_txout.outpoint.txid)
-                } else {
-                    None
-                }
-            })
-            .collect::<BTreeSet<_>>();
-
-        let unconfirmed_utxos_txid = utxos
-            .iter()
-            .filter_map(|full_txout| {
-                if matches!(full_txout.chain_position, ObservedAs::Unconfirmed(_)) {
-                    Some(full_txout.outpoint.txid)
-                } else {
-                    None
-                }
-            })
-            .collect::<BTreeSet<_>>();
-
-        (
-            confirmed_txouts_txid,
-            unconfirmed_txouts_txid,
-            confirmed_utxos_txid,
-            unconfirmed_utxos_txid,
-            balance,
-        )
-    };
+    let fetch =
+        |ht: u32, graph: &IndexedTxGraph<ConfirmationHeightAnchor, KeychainTxOutIndex<String>>| {
+            let txouts = graph
+                .list_owned_txouts(&local_chain, local_chain.get_block(ht).unwrap())
+                .collect::<Vec<_>>();
+
+            let utxos = graph
+                .list_owned_unspents(&local_chain, local_chain.get_block(ht).unwrap())
+                .collect::<Vec<_>>();
+
+            let balance = graph.balance(
+                &local_chain,
+                local_chain.get_block(ht).unwrap(),
+                |spk: &Script| trusted_spks.contains(spk),
+            );
+
+            assert_eq!(txouts.len(), 5);
+            assert_eq!(utxos.len(), 4);
+
+            let confirmed_txouts_txid = txouts
+                .iter()
+                .filter_map(|full_txout| {
+                    if matches!(full_txout.chain_position, ObservedAs::Confirmed(_)) {
+                        Some(full_txout.outpoint.txid)
+                    } else {
+                        None
+                    }
+                })
+                .collect::<BTreeSet<_>>();
+
+            let unconfirmed_txouts_txid = txouts
+                .iter()
+                .filter_map(|full_txout| {
+                    if matches!(full_txout.chain_position, ObservedAs::Unconfirmed(_)) {
+                        Some(full_txout.outpoint.txid)
+                    } else {
+                        None
+                    }
+                })
+                .collect::<BTreeSet<_>>();
+
+            let confirmed_utxos_txid = utxos
+                .iter()
+                .filter_map(|full_txout| {
+                    if matches!(full_txout.chain_position, ObservedAs::Confirmed(_)) {
+                        Some(full_txout.outpoint.txid)
+                    } else {
+                        None
+                    }
+                })
+                .collect::<BTreeSet<_>>();
+
+            let unconfirmed_utxos_txid = utxos
+                .iter()
+                .filter_map(|full_txout| {
+                    if matches!(full_txout.chain_position, ObservedAs::Unconfirmed(_)) {
+                        Some(full_txout.outpoint.txid)
+                    } else {
+                        None
+                    }
+                })
+                .collect::<BTreeSet<_>>();
+
+            (
+                confirmed_txouts_txid,
+                unconfirmed_txouts_txid,
+                confirmed_utxos_txid,
+                unconfirmed_utxos_txid,
+                balance,
+            )
+        };
 
     // ----- TEST BLOCK -----
 
index 7e8c3ad099c3ec901119e5eb174904e58667d2fb..427de71e7a57425cfb71ad080fe01cee49d87872 100644 (file)
@@ -4,7 +4,7 @@ use bdk_chain::{
     collections::*,
     local_chain::LocalChain,
     tx_graph::{Additions, TxGraph},
-    Append, BlockId, ObservedAs,
+    Append, BlockId, ConfirmationHeightAnchor, ObservedAs,
 };
 use bitcoin::{
     hashes::Hash, BlockHash, OutPoint, PackedLockTime, Script, Transaction, TxIn, TxOut, Txid,
@@ -684,7 +684,7 @@ fn test_chain_spends() {
         ..common::new_tx(0)
     };
 
-    let mut graph = TxGraph::<BlockId>::default();
+    let mut graph = TxGraph::<ConfirmationHeightAnchor>::default();
 
     let _ = graph.insert_tx(tx_0.clone());
     let _ = graph.insert_tx(tx_1.clone());
@@ -694,27 +694,36 @@ fn test_chain_spends() {
         .iter()
         .zip([&tx_0, &tx_1].into_iter())
         .for_each(|(ht, tx)| {
-            let block_id = local_chain.get_block(*ht).expect("block expected");
-            let _ = graph.insert_anchor(tx.txid(), block_id);
+            // let block_id = local_chain.get_block(*ht).expect("block expected");
+            let _ = graph.insert_anchor(
+                tx.txid(),
+                ConfirmationHeightAnchor {
+                    anchor_block: tip,
+                    confirmation_height: *ht,
+                },
+            );
         });
 
     // Assert that confirmed spends are returned correctly.
     assert_eq!(
-        graph
-            .get_chain_spend(&local_chain, tip, OutPoint::new(tx_0.txid(), 0))
-            .unwrap(),
-        (
-            ObservedAs::Confirmed(&local_chain.get_block(98).expect("block expected")),
-            tx_1.txid()
-        )
+        graph.get_chain_spend(&local_chain, tip, OutPoint::new(tx_0.txid(), 0)),
+        Some((
+            ObservedAs::Confirmed(&ConfirmationHeightAnchor {
+                anchor_block: tip,
+                confirmation_height: 98
+            }),
+            tx_1.txid(),
+        )),
     );
 
     // Check if chain position is returned correctly.
     assert_eq!(
-        graph
-            .get_chain_position(&local_chain, tip, tx_0.txid())
-            .expect("position expected"),
-        ObservedAs::Confirmed(&local_chain.get_block(95).expect("block expected"))
+        graph.get_chain_position(&local_chain, tip, tx_0.txid()),
+        // Some(ObservedAs::Confirmed(&local_chain.get_block(95).expect("block expected"))),
+        Some(ObservedAs::Confirmed(&ConfirmationHeightAnchor {
+            anchor_block: tip,
+            confirmation_height: 95
+        }))
     );
 
     // Even if unconfirmed tx has a last_seen of 0, it can still be part of a chain spend.
@@ -784,73 +793,6 @@ fn test_chain_spends() {
         .is_none());
 }
 
-#[test]
-fn test_relevant_heights() {
-    let mut graph = TxGraph::<BlockId>::default();
-
-    let tx1 = common::new_tx(1);
-    let tx2 = common::new_tx(2);
-
-    let _ = graph.insert_tx(tx1.clone());
-    assert_eq!(
-        graph.relevant_heights().collect::<Vec<_>>(),
-        vec![],
-        "no anchors in graph"
-    );
-
-    let _ = graph.insert_anchor(
-        tx1.txid(),
-        BlockId {
-            height: 3,
-            hash: h!("3a"),
-        },
-    );
-    assert_eq!(
-        graph.relevant_heights().collect::<Vec<_>>(),
-        vec![3],
-        "one anchor at height 3"
-    );
-
-    let _ = graph.insert_anchor(
-        tx1.txid(),
-        BlockId {
-            height: 3,
-            hash: h!("3b"),
-        },
-    );
-    assert_eq!(
-        graph.relevant_heights().collect::<Vec<_>>(),
-        vec![3],
-        "introducing duplicate anchor at height 3, must not iterate over duplicate heights"
-    );
-
-    let _ = graph.insert_anchor(
-        tx1.txid(),
-        BlockId {
-            height: 4,
-            hash: h!("4a"),
-        },
-    );
-    assert_eq!(
-        graph.relevant_heights().collect::<Vec<_>>(),
-        vec![3, 4],
-        "anchors in height 3 and now 4"
-    );
-
-    let _ = graph.insert_anchor(
-        tx2.txid(),
-        BlockId {
-            height: 5,
-            hash: h!("5a"),
-        },
-    );
-    assert_eq!(
-        graph.relevant_heights().collect::<Vec<_>>(),
-        vec![3, 4, 5],
-        "anchor for non-existant tx is inserted at height 5, must still be in relevant heights",
-    );
-}
-
 /// Ensure that `last_seen` values only increase during [`Append::append`].
 #[test]
 fn test_additions_last_seen_append() {