From 8de421f46bc838c6597bdd4765c2e82a2c21a43b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Wed, 3 May 2023 16:03:23 +0800 Subject: [PATCH] [chain_redesign] `BlockId` should not implement `Anchor` 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 | 58 ++++++- crates/chain/src/tx_graph.rs | 20 +-- crates/chain/tests/test_indexed_tx_graph.rs | 163 +++++++++++--------- crates/chain/tests/test_tx_graph.rs | 106 +++---------- 4 files changed, 167 insertions(+), 180 deletions(-) diff --git a/crates/chain/src/chain_data.rs b/crates/chain/src/chain_data.rs index a360b304e0..3252febc25 100644 --- a/crates/chain/src/chain_data.rs +++ b/crates/chain/src/chain_data.rs @@ -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

{ diff --git a/crates/chain/src/tx_graph.rs b/crates/chain/src/tx_graph.rs index d8b1303030..b1152c334f 100644 --- a/crates/chain/src/tx_graph.rs +++ b/crates/chain/src/tx_graph.rs @@ -349,6 +349,11 @@ impl TxGraph { .filter(move |(_, conflicting_txid)| *conflicting_txid != txid) } + /// Iterates over all transaction anchors known by [`TxGraph`]. + pub fn all_anchors(&self) -> impl ExactSizeIterator + DoubleEndedIterator { + self.anchors.iter() + } + /// 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 TxGraph { } impl TxGraph { - /// Get all heights that are relevant to the graph. - pub fn relevant_heights(&self) -> impl Iterator + '_ { - let mut last_height = Option::::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 diff --git a/crates/chain/tests/test_indexed_tx_graph.rs b/crates/chain/tests/test_indexed_tx_graph.rs index 78f3c3a649..d88c1e3983 100644 --- a/crates/chain/tests/test_indexed_tx_graph.rs +++ b/crates/chain/tests/test_indexed_tx_graph.rs @@ -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::>::default(); + let mut graph = IndexedTxGraph::>::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::>::default(); + let mut graph = + IndexedTxGraph::>::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>| { - let txouts = graph - .list_owned_txouts(&local_chain, local_chain.get_block(ht).unwrap()) - .collect::>(); - - let utxos = graph - .list_owned_unspents(&local_chain, local_chain.get_block(ht).unwrap()) - .collect::>(); - - 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::>(); - - 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::>(); - - 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::>(); - - 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::>(); - - ( - confirmed_txouts_txid, - unconfirmed_txouts_txid, - confirmed_utxos_txid, - unconfirmed_utxos_txid, - balance, - ) - }; + let fetch = + |ht: u32, graph: &IndexedTxGraph>| { + let txouts = graph + .list_owned_txouts(&local_chain, local_chain.get_block(ht).unwrap()) + .collect::>(); + + let utxos = graph + .list_owned_unspents(&local_chain, local_chain.get_block(ht).unwrap()) + .collect::>(); + + 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::>(); + + 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::>(); + + 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::>(); + + 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::>(); + + ( + confirmed_txouts_txid, + unconfirmed_txouts_txid, + confirmed_utxos_txid, + unconfirmed_utxos_txid, + balance, + ) + }; // ----- TEST BLOCK ----- diff --git a/crates/chain/tests/test_tx_graph.rs b/crates/chain/tests/test_tx_graph.rs index 7e8c3ad099..427de71e7a 100644 --- a/crates/chain/tests/test_tx_graph.rs +++ b/crates/chain/tests/test_tx_graph.rs @@ -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::::default(); + let mut graph = TxGraph::::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::::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![], - "no anchors in graph" - ); - - let _ = graph.insert_anchor( - tx1.txid(), - BlockId { - height: 3, - hash: h!("3a"), - }, - ); - assert_eq!( - graph.relevant_heights().collect::>(), - 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![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![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![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() {