From 0aa39f9e1ca406c55a1e33477ec3806c7cf8b9b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Wed, 16 Oct 2024 14:28:23 +0000 Subject: [PATCH 01/16] feat(chain)!: `TxGraph` contain anchors in one field Previously, the field `TxGraph::anchors` existed because we assumed there was use in having a partially-chronological order of transactions there. Turns out it wasn't used at all. This commit removes anchors from the `txs` field and reworks `anchors` field to be a map of `txid -> set`. This is a breaking change since the signature of `all_anchors()` is changed. Update `TxGraph` field docs for `empty_outspends` and `empty_anchors`. --- crates/chain/src/tx_graph.rs | 75 +++++++++++++++-------------- crates/chain/tests/test_tx_graph.rs | 6 ++- 2 files changed, 42 insertions(+), 39 deletions(-) diff --git a/crates/chain/src/tx_graph.rs b/crates/chain/src/tx_graph.rs index b266cf9ea..444328f16 100644 --- a/crates/chain/src/tx_graph.rs +++ b/crates/chain/src/tx_graph.rs @@ -106,7 +106,7 @@ use core::{ ops::{Deref, RangeInclusive}, }; -impl From> for TxUpdate { +impl From> for TxUpdate { fn from(graph: TxGraph) -> Self { Self { txs: graph.full_txs().map(|tx_node| tx_node.tx).collect(), @@ -114,7 +114,11 @@ impl From> for TxUpdate { .floating_txouts() .map(|(op, txo)| (op, txo.clone())) .collect(), - anchors: graph.anchors, + anchors: graph + .anchors + .into_iter() + .flat_map(|(txid, anchors)| anchors.into_iter().map(move |a| (a, txid))) + .collect(), seen_ats: graph.last_seen.into_iter().collect(), } } @@ -135,15 +139,15 @@ impl From> for TxGraph { /// [module-level documentation]: crate::tx_graph #[derive(Clone, Debug, PartialEq)] pub struct TxGraph { - // all transactions that the graph is aware of in format: `(tx_node, tx_anchors)` - txs: HashMap)>, + txs: HashMap, spends: BTreeMap>, - anchors: BTreeSet<(A, Txid)>, + anchors: HashMap>, last_seen: HashMap, - // This atrocity exists so that `TxGraph::outspends()` can return a reference. - // FIXME: This can be removed once `HashSet::new` is a const fn. + // The following fields exist so that methods can return references to empty sets. + // FIXME: This can be removed once `HashSet::new` and `BTreeSet::new` are const fns. empty_outspends: HashSet, + empty_anchors: BTreeSet, } impl Default for TxGraph { @@ -154,6 +158,7 @@ impl Default for TxGraph { anchors: Default::default(), last_seen: Default::default(), empty_outspends: Default::default(), + empty_anchors: Default::default(), } } } @@ -238,7 +243,7 @@ impl TxGraph { /// /// This includes txouts of both full transactions as well as floating transactions. pub fn all_txouts(&self) -> impl Iterator { - self.txs.iter().flat_map(|(txid, (tx, _))| match tx { + self.txs.iter().flat_map(|(txid, tx)| match tx { TxNodeInternal::Whole(tx) => tx .as_ref() .output @@ -260,7 +265,7 @@ impl TxGraph { pub fn floating_txouts(&self) -> impl Iterator { self.txs .iter() - .filter_map(|(txid, (tx_node, _))| match tx_node { + .filter_map(|(txid, tx_node)| match tx_node { TxNodeInternal::Whole(_) => None, TxNodeInternal::Partial(txouts) => Some( txouts @@ -273,17 +278,15 @@ impl TxGraph { /// Iterate over all full transactions in the graph. pub fn full_txs(&self) -> impl Iterator, A>> { - self.txs - .iter() - .filter_map(|(&txid, (tx, anchors))| match tx { - TxNodeInternal::Whole(tx) => Some(TxNode { - txid, - tx: tx.clone(), - anchors, - last_seen_unconfirmed: self.last_seen.get(&txid).copied(), - }), - TxNodeInternal::Partial(_) => None, - }) + self.txs.iter().filter_map(|(&txid, tx)| match tx { + TxNodeInternal::Whole(tx) => Some(TxNode { + txid, + tx: tx.clone(), + anchors: self.anchors.get(&txid).unwrap_or(&self.empty_anchors), + last_seen_unconfirmed: self.last_seen.get(&txid).copied(), + }), + TxNodeInternal::Partial(_) => None, + }) } /// Iterate over graph transactions with no anchors or last-seen. @@ -311,10 +314,10 @@ impl TxGraph { /// Get a transaction node by txid. This only returns `Some` for full transactions. pub fn get_tx_node(&self, txid: Txid) -> Option, A>> { match &self.txs.get(&txid)? { - (TxNodeInternal::Whole(tx), anchors) => Some(TxNode { + TxNodeInternal::Whole(tx) => Some(TxNode { txid, tx: tx.clone(), - anchors, + anchors: self.anchors.get(&txid).unwrap_or(&self.empty_anchors), last_seen_unconfirmed: self.last_seen.get(&txid).copied(), }), _ => None, @@ -323,7 +326,7 @@ impl TxGraph { /// Obtains a single tx output (if any) at the specified outpoint. pub fn get_txout(&self, outpoint: OutPoint) -> Option<&TxOut> { - match &self.txs.get(&outpoint.txid)?.0 { + match &self.txs.get(&outpoint.txid)? { TxNodeInternal::Whole(tx) => tx.as_ref().output.get(outpoint.vout as usize), TxNodeInternal::Partial(txouts) => txouts.get(&outpoint.vout), } @@ -333,7 +336,7 @@ impl TxGraph { /// /// Returns a [`BTreeMap`] of vout to output of the provided `txid`. pub fn tx_outputs(&self, txid: Txid) -> Option> { - Some(match &self.txs.get(&txid)?.0 { + Some(match &self.txs.get(&txid)? { TxNodeInternal::Whole(tx) => tx .as_ref() .output @@ -496,7 +499,7 @@ impl TxGraph { } /// Get all transaction anchors known by [`TxGraph`]. - pub fn all_anchors(&self) -> &BTreeSet<(A, Txid)> { + pub fn all_anchors(&self) -> &HashMap> { &self.anchors } @@ -540,7 +543,7 @@ impl TxGraph { /// [`apply_changeset`]: Self::apply_changeset pub fn insert_txout(&mut self, outpoint: OutPoint, txout: TxOut) -> ChangeSet { let mut changeset = ChangeSet::::default(); - let (tx_node, _) = self.txs.entry(outpoint.txid).or_default(); + let tx_node = self.txs.entry(outpoint.txid).or_default(); match tx_node { TxNodeInternal::Whole(_) => { // ignore this txout we have the full one already. @@ -573,7 +576,7 @@ impl TxGraph { let txid = tx.compute_txid(); let mut changeset = ChangeSet::::default(); - let (tx_node, _) = self.txs.entry(txid).or_default(); + let tx_node = self.txs.entry(txid).or_default(); match tx_node { TxNodeInternal::Whole(existing_tx) => { debug_assert_eq!( @@ -625,13 +628,7 @@ impl TxGraph { /// `anchor`. pub fn insert_anchor(&mut self, txid: Txid, anchor: A) -> ChangeSet { let mut changeset = ChangeSet::::default(); - if self.anchors.insert((anchor.clone(), txid)) { - let (_tx_node, anchors) = self.txs.entry(txid).or_default(); - let _inserted = anchors.insert(anchor.clone()); - debug_assert!( - _inserted, - "anchors in `.anchors` and `.txs` should be consistent" - ); + if self.anchors.entry(txid).or_default().insert(anchor.clone()) { changeset.anchors.insert((anchor, txid)); } changeset @@ -711,7 +708,11 @@ impl TxGraph { .floating_txouts() .map(|(op, txout)| (op, txout.clone())) .collect(), - anchors: self.anchors.clone(), + anchors: self + .anchors + .iter() + .flat_map(|(txid, anchors)| anchors.iter().map(|a| (a.clone(), *txid))) + .collect(), last_seen: self.last_seen.iter().map(|(&k, &v)| (k, v)).collect(), } } @@ -763,12 +764,12 @@ impl TxGraph { chain_tip: BlockId, txid: Txid, ) -> Result>, C::Error> { - let (tx_node, anchors) = match self.txs.get(&txid) { + let tx_node = match self.txs.get(&txid) { Some(v) => v, None => return Ok(None), }; - for anchor in anchors { + for anchor in self.anchors.get(&txid).unwrap_or(&self.empty_anchors) { match chain.is_block_in_chain(anchor.anchor_block(), chain_tip)? { Some(true) => { return Ok(Some(ChainPosition::Confirmed { diff --git a/crates/chain/tests/test_tx_graph.rs b/crates/chain/tests/test_tx_graph.rs index 05d5d63e4..8aff2d40c 100644 --- a/crates/chain/tests/test_tx_graph.rs +++ b/crates/chain/tests/test_tx_graph.rs @@ -1168,11 +1168,13 @@ fn call_map_anchors_with_non_deterministic_anchor() { } assert!(new_txs.next().is_none()); - let new_graph_anchors: Vec<_> = new_graph + let mut new_graph_anchors: Vec<_> = new_graph .all_anchors() .iter() - .map(|i| i.0.anchor_block) + .flat_map(|(_, anchors)| anchors) + .map(|a| a.anchor_block) .collect(); + new_graph_anchors.sort(); assert_eq!( new_graph_anchors, vec![ From f6192a615a7aea2ca9bdcca199356ea4d0386ce1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 26 Nov 2024 12:36:59 +1100 Subject: [PATCH 02/16] feat(chain)!: Add `run_until_finished` methods Add `run_until_finished` methods for `TxAncestors` and `TxDescendants`. This is useful for traversing until the internal closure returns `None`. Signatures of `TxAncestors` and `TxDescendants` are changed to enforce generic bounds in the type definition. --- crates/chain/src/tx_graph.rs | 42 ++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/crates/chain/src/tx_graph.rs b/crates/chain/src/tx_graph.rs index 444328f16..f00ab9a8c 100644 --- a/crates/chain/src/tx_graph.rs +++ b/crates/chain/src/tx_graph.rs @@ -435,7 +435,7 @@ impl TxGraph { /// /// The supplied closure returns an `Option`, allowing the caller to map each `Transaction` /// it visits and decide whether to visit ancestors. - pub fn walk_ancestors<'g, T, F, O>(&'g self, tx: T, walk_map: F) -> TxAncestors<'g, A, F> + pub fn walk_ancestors<'g, T, F, O>(&'g self, tx: T, walk_map: F) -> TxAncestors<'g, A, F, O> where T: Into>, F: FnMut(usize, Arc) -> Option + 'g, @@ -453,7 +453,7 @@ impl TxGraph { /// /// The supplied closure returns an `Option`, allowing the caller to map each node it visits /// and decide whether to visit descendants. - pub fn walk_descendants<'g, F, O>(&'g self, txid: Txid, walk_map: F) -> TxDescendants + pub fn walk_descendants<'g, F, O>(&'g self, txid: Txid, walk_map: F) -> TxDescendants where F: FnMut(usize, Txid) -> Option + 'g, { @@ -470,7 +470,7 @@ impl TxGraph { &'g self, tx: &'g Transaction, walk_map: F, - ) -> TxDescendants + ) -> TxDescendants where F: FnMut(usize, Txid) -> Option + 'g, { @@ -1329,14 +1329,20 @@ impl AsRef> for TxGraph { /// Returned by the [`walk_ancestors`] method of [`TxGraph`]. /// /// [`walk_ancestors`]: TxGraph::walk_ancestors -pub struct TxAncestors<'g, A, F> { +pub struct TxAncestors<'g, A, F, O> +where + F: FnMut(usize, Arc) -> Option, +{ graph: &'g TxGraph, visited: HashSet, queue: VecDeque<(usize, Arc)>, filter_map: F, } -impl<'g, A, F> TxAncestors<'g, A, F> { +impl<'g, A, F, O> TxAncestors<'g, A, F, O> +where + F: FnMut(usize, Arc) -> Option, +{ /// Creates a `TxAncestors` that includes the starting `Transaction` when iterating. pub(crate) fn new_include_root( graph: &'g TxGraph, @@ -1411,6 +1417,11 @@ impl<'g, A, F> TxAncestors<'g, A, F> { ancestors } + /// Traverse all ancestors that are not filtered out by the provided closure. + pub fn run_until_finished(self) { + self.for_each(|_| {}) + } + fn populate_queue(&mut self, depth: usize, tx: Arc) { let ancestors = tx .input @@ -1423,7 +1434,7 @@ impl<'g, A, F> TxAncestors<'g, A, F> { } } -impl<'g, A, F, O> Iterator for TxAncestors<'g, A, F> +impl<'g, A, F, O> Iterator for TxAncestors<'g, A, F, O> where F: FnMut(usize, Arc) -> Option, { @@ -1449,14 +1460,20 @@ where /// Returned by the [`walk_descendants`] method of [`TxGraph`]. /// /// [`walk_descendants`]: TxGraph::walk_descendants -pub struct TxDescendants<'g, A, F> { +pub struct TxDescendants<'g, A, F, O> +where + F: FnMut(usize, Txid) -> Option, +{ graph: &'g TxGraph, visited: HashSet, queue: VecDeque<(usize, Txid)>, filter_map: F, } -impl<'g, A, F> TxDescendants<'g, A, F> { +impl<'g, A, F, O> TxDescendants<'g, A, F, O> +where + F: FnMut(usize, Txid) -> Option, +{ /// Creates a `TxDescendants` that includes the starting `txid` when iterating. #[allow(unused)] pub(crate) fn new_include_root(graph: &'g TxGraph, txid: Txid, filter_map: F) -> Self { @@ -1520,9 +1537,12 @@ impl<'g, A, F> TxDescendants<'g, A, F> { } descendants } -} -impl<'g, A, F> TxDescendants<'g, A, F> { + /// Traverse all descendants that are not filtered out by the provided closure. + pub fn run_until_finished(self) { + self.for_each(|_| {}) + } + fn populate_queue(&mut self, depth: usize, txid: Txid) { let spend_paths = self .graph @@ -1534,7 +1554,7 @@ impl<'g, A, F> TxDescendants<'g, A, F> { } } -impl<'g, A, F, O> Iterator for TxDescendants<'g, A, F> +impl<'g, A, F, O> Iterator for TxDescendants<'g, A, F, O> where F: FnMut(usize, Txid) -> Option, { From 582d6b51f865d98077d72422d15630a2a3c7d16e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Sun, 3 Nov 2024 12:51:58 +1100 Subject: [PATCH 03/16] feat(chain)!: `O(n)` canonicalization algorithm This is an O(n) algorithm to determine the canonical set of txids. * Run 1: Iterate txs with anchors, starting from highest anchor height txs. * Run 2: Iterate txs with last-seen values, starting from highest last-seen values. * Run 3: Iterate txs that are remaining from run 1 which are not anchored in the best chain. Since all transitively-anchored txs are added to the `canonical` set in run 1, and anything that conflicts to anchored txs are already added to `not_canonial`, we can guarantee that run 2 will not traverse anything that directly or indirectly conflicts anything that is anchored. Run 3 is needed in case a tx does not have a last-seen value, but is seen in a conflicting chain. `TxGraph` is updated to include indexes `txids_by_anchor_height` and `txids_by_last_seen`. These are populated by the `insert_anchor` and `insert_seen_at` methods. Generic constaints needed to be tightened as these methods need to be aware of the anchor height to create `LastSeenIn`. --- crates/bitcoind_rpc/tests/test_emitter.rs | 1 + crates/chain/src/canonical_iter.rs | 249 +++++++++++++++++ crates/chain/src/lib.rs | 2 + crates/chain/src/tx_graph.rs | 258 +++++++++++++----- crates/chain/tests/common/tx_template.rs | 4 +- crates/chain/tests/test_indexed_tx_graph.rs | 47 ++-- crates/chain/tests/test_tx_graph.rs | 24 +- crates/chain/tests/test_tx_graph_conflicts.rs | 4 +- crates/wallet/src/test_utils.rs | 55 ++-- crates/wallet/src/wallet/mod.rs | 8 +- crates/wallet/tests/wallet.rs | 75 ++--- example-crates/example_cli/src/lib.rs | 36 +-- 12 files changed, 562 insertions(+), 201 deletions(-) create mode 100644 crates/chain/src/canonical_iter.rs diff --git a/crates/bitcoind_rpc/tests/test_emitter.rs b/crates/bitcoind_rpc/tests/test_emitter.rs index 8c41efc03..14b0c9212 100644 --- a/crates/bitcoind_rpc/tests/test_emitter.rs +++ b/crates/bitcoind_rpc/tests/test_emitter.rs @@ -389,6 +389,7 @@ fn tx_can_become_unconfirmed_after_reorg() -> anyhow::Result<()> { assert_eq!( get_balance(&recv_chain, &recv_graph)?, Balance { + trusted_pending: SEND_AMOUNT * reorg_count as u64, confirmed: SEND_AMOUNT * (ADDITIONAL_COUNT - reorg_count) as u64, ..Balance::default() }, diff --git a/crates/chain/src/canonical_iter.rs b/crates/chain/src/canonical_iter.rs new file mode 100644 index 000000000..7077e4721 --- /dev/null +++ b/crates/chain/src/canonical_iter.rs @@ -0,0 +1,249 @@ +use crate::collections::{hash_map, HashMap, HashSet, VecDeque}; +use crate::tx_graph::{TxAncestors, TxDescendants}; +use crate::{Anchor, ChainOracle, TxGraph}; +use alloc::boxed::Box; +use alloc::collections::BTreeSet; +use alloc::sync::Arc; +use bdk_core::BlockId; +use bitcoin::{Transaction, Txid}; + +/// Iterates over canonical txs. +pub struct CanonicalIter<'g, A, C> { + tx_graph: &'g TxGraph, + chain: &'g C, + chain_tip: BlockId, + + pending_anchored: Box, &'g BTreeSet)> + 'g>, + pending_last_seen: Box, u64)> + 'g>, + pending_remaining: VecDeque<(Txid, Arc, u32)>, + + canonical: HashMap, CanonicalReason)>, + not_canonical: HashSet, + + queue: VecDeque, +} + +impl<'g, A: Anchor, C: ChainOracle> CanonicalIter<'g, A, C> { + /// Constructs [`CanonicalIter`]. + pub fn new(tx_graph: &'g TxGraph, chain: &'g C, chain_tip: BlockId) -> Self { + let anchors = tx_graph.all_anchors(); + let pending_anchored = Box::new( + tx_graph + .txids_by_descending_anchor_height() + .filter_map(|(_, txid)| Some((txid, tx_graph.get_tx(txid)?, anchors.get(&txid)?))), + ); + let pending_last_seen = Box::new( + tx_graph + .txids_by_descending_last_seen() + .filter_map(|(last_seen, txid)| Some((txid, tx_graph.get_tx(txid)?, last_seen))), + ); + Self { + tx_graph, + chain, + chain_tip, + pending_anchored, + pending_last_seen, + pending_remaining: VecDeque::new(), + canonical: HashMap::new(), + not_canonical: HashSet::new(), + queue: VecDeque::new(), + } + } + + /// Whether this transaction is already canonicalized. + fn is_canonicalized(&self, txid: Txid) -> bool { + self.canonical.contains_key(&txid) || self.not_canonical.contains(&txid) + } + + /// Mark transaction as canonical if it is anchored in the best chain. + fn scan_anchors( + &mut self, + txid: Txid, + tx: Arc, + anchors: &BTreeSet, + ) -> Result<(), C::Error> { + for anchor in anchors { + let in_chain_opt = self + .chain + .is_block_in_chain(anchor.anchor_block(), self.chain_tip)?; + if in_chain_opt == Some(true) { + self.mark_canonical(tx, CanonicalReason::from_anchor(anchor.clone())); + return Ok(()); + } + } + // cannot determine + self.pending_remaining.push_back(( + txid, + tx, + anchors + .iter() + .last() + .unwrap() + .confirmation_height_upper_bound(), + )); + Ok(()) + } + + /// Marks a transaction and it's ancestors as canoncial. Mark all conflicts of these as + /// `not_canonical`. + fn mark_canonical(&mut self, tx: Arc, reason: CanonicalReason) { + let starting_txid = tx.compute_txid(); + let mut is_root = true; + TxAncestors::new_include_root( + self.tx_graph, + tx, + |_: usize, tx: Arc| -> Option<()> { + let this_txid = tx.compute_txid(); + let this_reason = if is_root { + is_root = false; + reason.clone() + } else { + reason.to_transitive(starting_txid) + }; + let canonical_entry = match self.canonical.entry(this_txid) { + // Already visited tx before, exit early. + hash_map::Entry::Occupied(_) => return None, + hash_map::Entry::Vacant(entry) => entry, + }; + // Any conflicts with a canonical tx can be added to `not_canonical`. Descendants + // of `not_canonical` txs can also be added to `not_canonical`. + for (_, conflict_txid) in self.tx_graph.direct_conflicts(&tx) { + TxDescendants::new_include_root( + self.tx_graph, + conflict_txid, + |_: usize, txid: Txid| -> Option<()> { + if self.not_canonical.insert(txid) { + Some(()) + } else { + None + } + }, + ) + .run_until_finished() + } + canonical_entry.insert((tx, this_reason)); + self.queue.push_back(this_txid); + Some(()) + }, + ) + .for_each(|_| {}) + } +} + +impl<'g, A: Anchor, C: ChainOracle> Iterator for CanonicalIter<'g, A, C> { + type Item = Result<(Txid, Arc, CanonicalReason), C::Error>; + + fn next(&mut self) -> Option { + loop { + if let Some(txid) = self.queue.pop_front() { + let (tx, reason) = self + .canonical + .get(&txid) + .cloned() + .expect("reason must exist"); + return Some(Ok((txid, tx, reason))); + } + + if let Some((txid, tx, anchors)) = self.pending_anchored.next() { + if !self.is_canonicalized(txid) { + if let Err(err) = self.scan_anchors(txid, tx, anchors) { + return Some(Err(err)); + } + } + continue; + } + + if let Some((txid, tx, last_seen)) = self.pending_last_seen.next() { + if !self.is_canonicalized(txid) { + let lsi = LastSeenIn::Mempool(last_seen); + self.mark_canonical(tx, CanonicalReason::from_last_seen(lsi)); + } + continue; + } + + if let Some((txid, tx, height)) = self.pending_remaining.pop_front() { + if !self.is_canonicalized(txid) { + let lsi = LastSeenIn::Block(height); + self.mark_canonical(tx, CanonicalReason::from_last_seen(lsi)); + } + continue; + } + + return None; + } + } +} + +/// Represents when and where a given transaction is last seen. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, core::hash::Hash)] +#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] +pub enum LastSeenIn { + /// The transaction was last seen in a block of height. + Block(u32), + /// The transaction was last seen in the mempool at the given unix timestamp. + Mempool(u64), +} + +/// The reason why a transaction is canonical. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum CanonicalReason { + /// This transaction is anchored in the best chain by `A`, and therefore canonical. + Anchor { + /// The anchor that anchored the transaction in the chain. + anchor: A, + /// Whether the anchor is of the transaction's descendant. + descendant: Option, + }, + /// This transaction does not conflict with any other transaction with a more recent `last_seen` + /// value or one that is anchored in the best chain. + LastSeen { + /// The [`LastSeenIn`] value of the transaction. + last_seen: LastSeenIn, + /// Whether the [`LastSeenIn`] value is of the transaction's descendant. + descendant: Option, + }, +} + +impl CanonicalReason { + /// Constructs a [`CanonicalReason`] from an `anchor`. + pub fn from_anchor(anchor: A) -> Self { + Self::Anchor { + anchor, + descendant: None, + } + } + + /// Constructs a [`CanonicalReason`] from a `last_seen` value. + pub fn from_last_seen(last_seen: LastSeenIn) -> Self { + Self::LastSeen { + last_seen, + descendant: None, + } + } + + /// Contruct a new [`CanonicalReason`] from the original which is transitive to `descendant`. + /// + /// This signals that either the [`LastSeenIn`] or [`Anchor`] value belongs to the transaction's + /// descendant, but is transitively relevant. + pub fn to_transitive(&self, descendant: Txid) -> Self { + match self { + CanonicalReason::Anchor { anchor, .. } => Self::Anchor { + anchor: anchor.clone(), + descendant: Some(descendant), + }, + CanonicalReason::LastSeen { last_seen, .. } => Self::LastSeen { + last_seen: *last_seen, + descendant: Some(descendant), + }, + } + } + + /// This signals that either the [`LastSeenIn`] or [`Anchor`] value belongs to the transaction's + /// descendant. + pub fn descendant(&self) -> &Option { + match self { + CanonicalReason::Anchor { descendant, .. } => descendant, + CanonicalReason::LastSeen { descendant, .. } => descendant, + } + } +} diff --git a/crates/chain/src/lib.rs b/crates/chain/src/lib.rs index 557d53494..92a6d5c4e 100644 --- a/crates/chain/src/lib.rs +++ b/crates/chain/src/lib.rs @@ -43,6 +43,8 @@ pub mod tx_graph; pub use tx_graph::TxGraph; mod chain_oracle; pub use chain_oracle::*; +mod canonical_iter; +pub use canonical_iter::*; #[doc(hidden)] pub mod example_utils; diff --git a/crates/chain/src/tx_graph.rs b/crates/chain/src/tx_graph.rs index f00ab9a8c..0e7dd33fa 100644 --- a/crates/chain/src/tx_graph.rs +++ b/crates/chain/src/tx_graph.rs @@ -94,10 +94,14 @@ use crate::collections::*; use crate::BlockId; +use crate::CanonicalIter; +use crate::CanonicalReason; +use crate::LastSeenIn; use crate::{Anchor, Balance, ChainOracle, ChainPosition, FullTxOut, Merge}; use alloc::collections::vec_deque::VecDeque; use alloc::sync::Arc; use alloc::vec::Vec; +use bdk_core::ConfirmationBlockTime; pub use bdk_core::TxUpdate; use bitcoin::{Amount, OutPoint, ScriptBuf, SignedAmount, Transaction, TxOut, Txid}; use core::fmt::{self, Formatter}; @@ -124,7 +128,7 @@ impl From> for TxUpdate { } } -impl From> for TxGraph { +impl From> for TxGraph { fn from(update: TxUpdate) -> Self { let mut graph = TxGraph::::default(); let _ = graph.apply_update_at(update, None); @@ -138,12 +142,15 @@ impl From> for TxGraph { /// /// [module-level documentation]: crate::tx_graph #[derive(Clone, Debug, PartialEq)] -pub struct TxGraph { +pub struct TxGraph { txs: HashMap, spends: BTreeMap>, anchors: HashMap>, last_seen: HashMap, + txs_by_highest_conf_heights: BTreeSet<(u32, Txid)>, + txs_by_last_seen: BTreeSet<(u64, Txid)>, + // The following fields exist so that methods can return references to empty sets. // FIXME: This can be removed once `HashSet::new` and `BTreeSet::new` are const fns. empty_outspends: HashSet, @@ -157,6 +164,8 @@ impl Default for TxGraph { spends: Default::default(), anchors: Default::default(), last_seen: Default::default(), + txs_by_highest_conf_heights: Default::default(), + txs_by_last_seen: Default::default(), empty_outspends: Default::default(), empty_anchors: Default::default(), } @@ -204,7 +213,7 @@ impl Default for TxNodeInternal { #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] pub struct CanonicalTx<'a, T, A> { /// How the transaction is observed as (confirmed or unconfirmed). - pub chain_position: ChainPosition<&'a A>, + pub chain_position: ChainPosition, /// The transaction node (as part of the graph). pub tx_node: TxNode<'a, T, A>, } @@ -509,12 +518,12 @@ impl TxGraph { } } -impl TxGraph { +impl TxGraph { /// Transform the [`TxGraph`] to have [`Anchor`]s of another type. /// /// This takes in a closure of signature `FnMut(A) -> A2` which is called for each [`Anchor`] to /// transform it. - pub fn map_anchors(self, f: F) -> TxGraph + pub fn map_anchors(self, f: F) -> TxGraph where F: FnMut(A) -> A2, { @@ -627,8 +636,36 @@ impl TxGraph { /// The [`ChangeSet`] returned will be empty if graph already knows that `txid` exists in /// `anchor`. pub fn insert_anchor(&mut self, txid: Txid, anchor: A) -> ChangeSet { + let mut old_top_h = None; + let mut new_top_h = anchor.confirmation_height_upper_bound(); + + let is_changed = match self.anchors.entry(txid) { + hash_map::Entry::Occupied(mut e) => { + fn top_anchor_height(anchors: &BTreeSet) -> Option { + anchors + .iter() + .last() + .map(Anchor::confirmation_height_upper_bound) + } + old_top_h = top_anchor_height(e.get()); + let is_changed = e.get_mut().insert(anchor.clone()); + new_top_h = top_anchor_height(e.get()).expect("must have anchor"); + is_changed + } + hash_map::Entry::Vacant(e) => { + e.insert(core::iter::once(anchor.clone()).collect()); + true + } + }; + let mut changeset = ChangeSet::::default(); - if self.anchors.entry(txid).or_default().insert(anchor.clone()) { + if is_changed { + if old_top_h != Some(new_top_h) { + if let Some(prev_top_h) = old_top_h { + self.txs_by_highest_conf_heights.remove(&(prev_top_h, txid)); + } + self.txs_by_highest_conf_heights.insert((new_top_h, txid)); + } changeset.anchors.insert((anchor, txid)); } changeset @@ -638,10 +675,29 @@ impl TxGraph { /// /// Note that [`TxGraph`] only keeps track of the latest `seen_at`. pub fn insert_seen_at(&mut self, txid: Txid, seen_at: u64) -> ChangeSet { + let mut old_last_seen = None; + let is_changed = match self.last_seen.entry(txid) { + hash_map::Entry::Occupied(mut e) => { + let last_seen = e.get_mut(); + old_last_seen = Some(*last_seen); + let change = *last_seen < seen_at; + if change { + *last_seen = seen_at; + } + change + } + hash_map::Entry::Vacant(e) => { + e.insert(seen_at); + true + } + }; + let mut changeset = ChangeSet::::default(); - let last_seen = self.last_seen.entry(txid).or_default(); - if seen_at > *last_seen { - *last_seen = seen_at; + if is_changed { + if let Some(old_last_seen) = old_last_seen { + self.txs_by_last_seen.remove(&(old_last_seen, txid)); + } + self.txs_by_last_seen.insert((seen_at, txid)); changeset.last_seen.insert(txid, seen_at); } changeset @@ -971,15 +1027,51 @@ impl TxGraph { chain: &'a C, chain_tip: BlockId, ) -> impl Iterator, A>, C::Error>> { - self.full_txs().filter_map(move |tx| { - self.try_get_chain_position(chain, chain_tip, tx.txid) - .map(|v| { - v.map(|observed_in| CanonicalTx { - chain_position: observed_in, - tx_node: tx, - }) + self.canonical_iter(chain, chain_tip).flat_map(move |res| { + res.map(|(txid, _, canonical_reason)| { + let tx_node = self.get_tx_node(txid).expect("must contain tx"); + let chain_position = match canonical_reason { + CanonicalReason::Anchor { anchor, descendant } => match descendant { + Some(_) => { + let direct_anchor = tx_node + .anchors + .iter() + .find_map(|a| -> Option> { + match chain.is_block_in_chain(a.anchor_block(), chain_tip) { + Ok(Some(true)) => Some(Ok(a.clone())), + Ok(Some(false)) | Ok(None) => None, + Err(err) => Some(Err(err)), + } + }) + .transpose()?; + match direct_anchor { + Some(anchor) => ChainPosition::Confirmed { + anchor, + transitively: None, + }, + None => ChainPosition::Confirmed { + anchor, + transitively: descendant, + }, + } + } + None => ChainPosition::Confirmed { + anchor, + transitively: None, + }, + }, + CanonicalReason::LastSeen { last_seen, .. } => match last_seen { + LastSeenIn::Mempool(last_seen) => ChainPosition::Unconfirmed { + last_seen: Some(last_seen), + }, + LastSeenIn::Block(_) => ChainPosition::Unconfirmed { last_seen: None }, + }, + }; + Ok(CanonicalTx { + chain_position, + tx_node, }) - .transpose() + }) }) } @@ -994,7 +1086,7 @@ impl TxGraph { chain_tip: BlockId, ) -> impl Iterator, A>> { self.try_list_canonical_txs(chain, chain_tip) - .map(|r| r.expect("oracle is infallible")) + .map(|res| res.expect("infallible")) } /// Get a filtered list of outputs from the given `outpoints` that are in `chain` with @@ -1021,44 +1113,80 @@ impl TxGraph { chain: &'a C, chain_tip: BlockId, outpoints: impl IntoIterator + 'a, - ) -> impl Iterator), C::Error>> + 'a { - outpoints - .into_iter() - .map( - move |(spk_i, op)| -> Result)>, C::Error> { - let tx_node = match self.get_tx_node(op.txid) { - Some(n) => n, - None => return Ok(None), - }; - - let txout = match tx_node.tx.as_ref().output.get(op.vout as usize) { - Some(txout) => txout.clone(), - None => return Ok(None), - }; - - let chain_position = - match self.try_get_chain_position(chain, chain_tip, op.txid)? { - Some(pos) => pos.cloned(), - None => return Ok(None), - }; - - let spent_by = self - .try_get_chain_spend(chain, chain_tip, op)? - .map(|(a, txid)| (a.cloned(), txid)); - - Ok(Some(( - spk_i, - FullTxOut { - outpoint: op, - txout, - chain_position, - spent_by, - is_on_coinbase: tx_node.tx.is_coinbase(), - }, - ))) + ) -> Result)> + 'a, C::Error> { + let mut canon_txs = HashMap::, A>>::new(); + let mut canon_spends = HashMap::::new(); + for r in self.try_list_canonical_txs(chain, chain_tip) { + let canonical_tx = r?; + let txid = canonical_tx.tx_node.txid; + + if !canonical_tx.tx_node.tx.is_coinbase() { + for txin in &canonical_tx.tx_node.tx.input { + let _res = canon_spends.insert(txin.previous_output, txid); + assert!( + _res.is_none(), + "tried to replace {:?} with {:?}", + _res, + txid + ); + } + } + canon_txs.insert(txid, canonical_tx); + } + Ok(outpoints.into_iter().filter_map(move |(spk_i, outpoint)| { + let canon_tx = canon_txs.get(&outpoint.txid)?; + let txout = canon_tx + .tx_node + .tx + .output + .get(outpoint.vout as usize) + .cloned()?; + let chain_position = canon_tx.chain_position.clone(); + let spent_by = canon_spends.get(&outpoint).map(|spend_txid| { + let spend_tx = canon_txs + .get(spend_txid) + .cloned() + .expect("must be canonical"); + (spend_tx.chain_position, *spend_txid) + }); + let is_on_coinbase = canon_tx.tx_node.is_coinbase(); + Some(( + spk_i, + FullTxOut { + outpoint, + txout, + chain_position, + spent_by, + is_on_coinbase, }, - ) - .filter_map(Result::transpose) + )) + })) + } + + /// List txids by descending anchor height order. + /// + /// If multiple anchors exist for a txid, the highest anchor height will be used. Transactions + /// without anchors are excluded. + pub fn txids_by_descending_anchor_height( + &self, + ) -> impl ExactSizeIterator + '_ { + self.txs_by_highest_conf_heights.iter().copied().rev() + } + + /// List txids by descending last-seen order. + /// + /// Transactions without last-seens are excluded. + pub fn txids_by_descending_last_seen(&self) -> impl ExactSizeIterator + '_ { + self.txs_by_last_seen.iter().copied().rev() + } + + /// Returns a [`CanonicalIter`]. + pub fn canonical_iter<'a, C: ChainOracle>( + &'a self, + chain: &'a C, + chain_tip: BlockId, + ) -> CanonicalIter<'a, A, C> { + CanonicalIter::new(self, chain, chain_tip) } /// Get a filtered list of outputs from the given `outpoints` that are in `chain` with @@ -1074,7 +1202,7 @@ impl TxGraph { outpoints: impl IntoIterator + 'a, ) -> impl Iterator)> + 'a { self.try_filter_chain_txouts(chain, chain_tip, outpoints) - .map(|r| r.expect("oracle is infallible")) + .expect("oracle is infallible") } /// Get a filtered list of unspent outputs (UTXOs) from the given `outpoints` that are in @@ -1100,14 +1228,10 @@ impl TxGraph { chain: &'a C, chain_tip: BlockId, outpoints: impl IntoIterator + 'a, - ) -> impl Iterator), C::Error>> + 'a { - self.try_filter_chain_txouts(chain, chain_tip, outpoints) - .filter(|r| match r { - // keep unspents, drop spents - Ok((_, full_txo)) => full_txo.spent_by.is_none(), - // keep errors - Err(_) => true, - }) + ) -> Result)> + 'a, C::Error> { + Ok(self + .try_filter_chain_txouts(chain, chain_tip, outpoints)? + .filter(|(_, full_txo)| full_txo.spent_by.is_none())) } /// Get a filtered list of unspent outputs (UTXOs) from the given `outpoints` that are in @@ -1123,7 +1247,7 @@ impl TxGraph { txouts: impl IntoIterator + 'a, ) -> impl Iterator)> + 'a { self.try_filter_chain_unspents(chain, chain_tip, txouts) - .map(|r| r.expect("oracle is infallible")) + .expect("oracle is infallible") } /// Get the total balance of `outpoints` that are in `chain` of `chain_tip`. @@ -1150,9 +1274,7 @@ impl TxGraph { let mut untrusted_pending = Amount::ZERO; let mut confirmed = Amount::ZERO; - for res in self.try_filter_chain_unspents(chain, chain_tip, outpoints) { - let (spk_i, txout) = res?; - + for (spk_i, txout) in self.try_filter_chain_unspents(chain, chain_tip, outpoints)? { match &txout.chain_position { ChainPosition::Confirmed { .. } => { if txout.is_confirmed_and_spendable(chain_tip.height) { diff --git a/crates/chain/tests/common/tx_template.rs b/crates/chain/tests/common/tx_template.rs index 6ece64cbb..0b0e2fd9e 100644 --- a/crates/chain/tests/common/tx_template.rs +++ b/crates/chain/tests/common/tx_template.rs @@ -132,7 +132,9 @@ pub fn init_graph<'a, A: Anchor + Clone + 'a>( for anchor in tx_tmp.anchors.iter() { let _ = graph.insert_anchor(tx.compute_txid(), anchor.clone()); } - let _ = graph.insert_seen_at(tx.compute_txid(), tx_tmp.last_seen.unwrap_or(0)); + if let Some(last_seen) = tx_tmp.last_seen { + let _ = graph.insert_seen_at(tx.compute_txid(), last_seen); + } } (graph, spk_index, tx_ids) } diff --git a/crates/chain/tests/test_indexed_tx_graph.rs b/crates/chain/tests/test_indexed_tx_graph.rs index 1b3dff573..e37a639c2 100644 --- a/crates/chain/tests/test_indexed_tx_graph.rs +++ b/crates/chain/tests/test_indexed_tx_graph.rs @@ -191,7 +191,7 @@ fn test_list_owned_txouts() { value: Amount::from_sat(70000), script_pubkey: trusted_spks[0].to_owned(), }], - ..new_tx(0) + ..new_tx(1) }; // tx2 is an incoming transaction received at untrusted keychain at block 1. @@ -200,7 +200,7 @@ fn test_list_owned_txouts() { value: Amount::from_sat(30000), script_pubkey: untrusted_spks[0].to_owned(), }], - ..new_tx(0) + ..new_tx(2) }; // tx3 spends tx2 and gives a change back in trusted keychain. Confirmed at Block 2. @@ -213,7 +213,7 @@ fn test_list_owned_txouts() { value: Amount::from_sat(10000), script_pubkey: trusted_spks[1].to_owned(), }], - ..new_tx(0) + ..new_tx(3) }; // tx4 is an external transaction receiving at untrusted keychain, unconfirmed. @@ -222,7 +222,7 @@ fn test_list_owned_txouts() { value: Amount::from_sat(20000), script_pubkey: untrusted_spks[1].to_owned(), }], - ..new_tx(0) + ..new_tx(4) }; // tx5 is an external transaction receiving at trusted keychain, unconfirmed. @@ -231,11 +231,12 @@ fn test_list_owned_txouts() { value: Amount::from_sat(15000), script_pubkey: trusted_spks[2].to_owned(), }], - ..new_tx(0) + ..new_tx(5) }; // tx6 is an unrelated transaction confirmed at 3. - let tx6 = new_tx(0); + // This won't be inserted because it is not relevant. + let tx6 = new_tx(6); // Insert transactions into graph with respective anchors // Insert unconfirmed txs with a last_seen timestamp @@ -293,7 +294,7 @@ fn test_list_owned_txouts() { let confirmed_txouts_txid = txouts .iter() .filter_map(|(_, full_txout)| { - if matches!(full_txout.chain_position, ChainPosition::Confirmed { .. }) { + if full_txout.chain_position.is_confirmed() { Some(full_txout.outpoint.txid) } else { None @@ -304,7 +305,7 @@ fn test_list_owned_txouts() { let unconfirmed_txouts_txid = txouts .iter() .filter_map(|(_, full_txout)| { - if matches!(full_txout.chain_position, ChainPosition::Unconfirmed { .. }) { + if !full_txout.chain_position.is_confirmed() { Some(full_txout.outpoint.txid) } else { None @@ -315,7 +316,7 @@ fn test_list_owned_txouts() { let confirmed_utxos_txid = utxos .iter() .filter_map(|(_, full_txout)| { - if matches!(full_txout.chain_position, ChainPosition::Confirmed { .. }) { + if full_txout.chain_position.is_confirmed() { Some(full_txout.outpoint.txid) } else { None @@ -326,7 +327,7 @@ fn test_list_owned_txouts() { let unconfirmed_utxos_txid = utxos .iter() .filter_map(|(_, full_txout)| { - if matches!(full_txout.chain_position, ChainPosition::Unconfirmed { .. }) { + if !full_txout.chain_position.is_confirmed() { Some(full_txout.outpoint.txid) } else { None @@ -360,20 +361,26 @@ fn test_list_owned_txouts() { assert_eq!(confirmed_txouts_txid, [tx1.compute_txid()].into()); assert_eq!( unconfirmed_txouts_txid, - [tx4.compute_txid(), tx5.compute_txid()].into() + [ + tx2.compute_txid(), + tx3.compute_txid(), + tx4.compute_txid(), + tx5.compute_txid() + ] + .into() ); assert_eq!(confirmed_utxos_txid, [tx1.compute_txid()].into()); assert_eq!( unconfirmed_utxos_txid, - [tx4.compute_txid(), tx5.compute_txid()].into() + [tx3.compute_txid(), tx4.compute_txid(), tx5.compute_txid()].into() ); assert_eq!( balance, Balance { immature: Amount::from_sat(70000), // immature coinbase - trusted_pending: Amount::from_sat(15000), // tx5 + trusted_pending: Amount::from_sat(25000), // tx3, tx5 untrusted_pending: Amount::from_sat(20000), // tx4 confirmed: Amount::ZERO // Nothing is confirmed yet } @@ -397,26 +404,23 @@ fn test_list_owned_txouts() { ); assert_eq!( unconfirmed_txouts_txid, - [tx4.compute_txid(), tx5.compute_txid()].into() + [tx3.compute_txid(), tx4.compute_txid(), tx5.compute_txid()].into() ); // tx2 gets into confirmed utxos set - assert_eq!( - confirmed_utxos_txid, - [tx1.compute_txid(), tx2.compute_txid()].into() - ); + assert_eq!(confirmed_utxos_txid, [tx1.compute_txid()].into()); assert_eq!( unconfirmed_utxos_txid, - [tx4.compute_txid(), tx5.compute_txid()].into() + [tx3.compute_txid(), tx4.compute_txid(), tx5.compute_txid()].into() ); assert_eq!( balance, Balance { immature: Amount::from_sat(70000), // immature coinbase - trusted_pending: Amount::from_sat(15000), // tx5 + trusted_pending: Amount::from_sat(25000), // tx3, tx5 untrusted_pending: Amount::from_sat(20000), // tx4 - confirmed: Amount::from_sat(30_000) // tx2 got confirmed + confirmed: Amount::from_sat(0) // tx2 got confirmed (but spent by 3) } ); } @@ -534,6 +538,7 @@ fn test_get_chain_position() { use bdk_chain::spk_txout::SpkTxOutIndex; use bdk_chain::BlockId; + #[derive(Debug)] struct TestCase { name: &'static str, tx: Transaction, diff --git a/crates/chain/tests/test_tx_graph.rs b/crates/chain/tests/test_tx_graph.rs index 8aff2d40c..8f1634ba1 100644 --- a/crates/chain/tests/test_tx_graph.rs +++ b/crates/chain/tests/test_tx_graph.rs @@ -185,7 +185,7 @@ fn insert_tx_graph_doesnt_count_coinbase_as_spent() { output: vec![], }; - let mut graph = TxGraph::<()>::default(); + let mut graph = TxGraph::::default(); let changeset = graph.insert_tx(tx); assert!(!changeset.is_empty()); assert!(graph.outspends(OutPoint::null()).is_empty()); @@ -216,8 +216,8 @@ fn insert_tx_graph_keeps_track_of_spend() { output: vec![], }; - let mut graph1 = TxGraph::<()>::default(); - let mut graph2 = TxGraph::<()>::default(); + let mut graph1 = TxGraph::::default(); + let mut graph2 = TxGraph::::default(); // insert in different order let _ = graph1.insert_tx(tx1.clone()); @@ -245,7 +245,7 @@ fn insert_tx_can_retrieve_full_tx_from_graph() { output: vec![TxOut::NULL], }; - let mut graph = TxGraph::<()>::default(); + let mut graph = TxGraph::::default(); let _ = graph.insert_tx(tx.clone()); assert_eq!( graph @@ -257,7 +257,7 @@ fn insert_tx_can_retrieve_full_tx_from_graph() { #[test] fn insert_tx_displaces_txouts() { - let mut tx_graph = TxGraph::<()>::default(); + let mut tx_graph = TxGraph::::default(); let tx = Transaction { version: transaction::Version::ONE, @@ -284,7 +284,7 @@ fn insert_tx_displaces_txouts() { #[test] fn insert_txout_does_not_displace_tx() { - let mut tx_graph = TxGraph::<()>::default(); + let mut tx_graph = TxGraph::::default(); let tx = Transaction { version: transaction::Version::ONE, lock_time: absolute::LockTime::ZERO, @@ -340,7 +340,7 @@ fn insert_txout_does_not_displace_tx() { #[test] fn test_calculate_fee() { - let mut graph = TxGraph::<()>::default(); + let mut graph = TxGraph::::default(); let intx1 = Transaction { version: transaction::Version::ONE, lock_time: absolute::LockTime::ZERO, @@ -694,7 +694,7 @@ fn test_conflicting_descendants() { let txid_a = tx_a.compute_txid(); let txid_b = tx_b.compute_txid(); - let mut graph = TxGraph::<()>::default(); + let mut graph = TxGraph::::default(); let _ = graph.insert_tx(tx_a); let _ = graph.insert_tx(tx_b); @@ -770,7 +770,7 @@ fn test_descendants_no_repeat() { }) .collect::>(); - let mut graph = TxGraph::<()>::default(); + let mut graph = TxGraph::::default(); let mut expected_txids = Vec::new(); // these are NOT descendants of `tx_a` @@ -1112,6 +1112,12 @@ fn call_map_anchors_with_non_deterministic_anchor() { pub non_deterministic_field: u32, } + impl Anchor for NonDeterministicAnchor { + fn anchor_block(&self) -> BlockId { + self.anchor_block + } + } + let template = [ TxTemplate { tx_name: "tx1", diff --git a/crates/chain/tests/test_tx_graph_conflicts.rs b/crates/chain/tests/test_tx_graph_conflicts.rs index 1f54c4b82..2e64e3912 100644 --- a/crates/chain/tests/test_tx_graph_conflicts.rs +++ b/crates/chain/tests/test_tx_graph_conflicts.rs @@ -413,12 +413,13 @@ fn test_tx_conflict_handling() { inputs: &[TxInTemplate::Bogus], outputs: &[TxOutTemplate::new(10000, Some(0))], anchors: &[block_id!(1, "B")], - last_seen: None, + ..Default::default() }, TxTemplate { tx_name: "B", inputs: &[TxInTemplate::PrevTx("A", 0)], outputs: &[TxOutTemplate::new(20000, Some(1))], + last_seen: Some(2), ..Default::default() }, TxTemplate { @@ -432,6 +433,7 @@ fn test_tx_conflict_handling() { tx_name: "C", inputs: &[TxInTemplate::PrevTx("B'", 0)], outputs: &[TxOutTemplate::new(30000, Some(3))], + last_seen: Some(1), ..Default::default() }, ], diff --git a/crates/wallet/src/test_utils.rs b/crates/wallet/src/test_utils.rs index c69de620a..7ad93e0c3 100644 --- a/crates/wallet/src/test_utils.rs +++ b/crates/wallet/src/test_utils.rs @@ -4,7 +4,7 @@ use alloc::string::ToString; use alloc::sync::Arc; use core::str::FromStr; -use bdk_chain::{tx_graph, BlockId, ChainPosition, ConfirmationBlockTime}; +use bdk_chain::{tx_graph, BlockId, ConfirmationBlockTime}; use bitcoin::{ absolute, hashes::Hash, transaction, Address, Amount, BlockHash, FeeRate, Network, OutPoint, Transaction, TxIn, TxOut, Txid, @@ -224,32 +224,43 @@ pub fn feerate_unchecked(sat_vb: f64) -> FeeRate { FeeRate::from_sat_per_kwu(sat_kwu) } +/// Input parameter for [`receive_output`]. +pub enum ReceiveTo { + /// Receive tx to mempool at this `last_seen` timestamp. + Mempool(u64), + /// Receive tx to block with this anchor. + Block(ConfirmationBlockTime), +} + +impl From for ReceiveTo { + fn from(value: ConfirmationBlockTime) -> Self { + Self::Block(value) + } +} + /// Receive a tx output with the given value in the latest block pub fn receive_output_in_latest_block(wallet: &mut Wallet, value: u64) -> OutPoint { let latest_cp = wallet.latest_checkpoint(); let height = latest_cp.height(); - let anchor = if height == 0 { - ChainPosition::Unconfirmed { last_seen: Some(0) } - } else { - ChainPosition::Confirmed { - anchor: ConfirmationBlockTime { - block_id: latest_cp.block_id(), - confirmation_time: 0, - }, - transitively: None, - } - }; - receive_output(wallet, value, anchor) + assert!(height > 0, "cannot receive tx into genesis block"); + receive_output( + wallet, + value, + ConfirmationBlockTime { + block_id: latest_cp.block_id(), + confirmation_time: 0, + }, + ) } /// Receive a tx output with the given value and chain position pub fn receive_output( wallet: &mut Wallet, value: u64, - pos: ChainPosition, + receive_to: impl Into, ) -> OutPoint { let addr = wallet.next_unused_address(KeychainKind::External).address; - receive_output_to_address(wallet, addr, value, pos) + receive_output_to_address(wallet, addr, value, receive_to) } /// Receive a tx output to an address with the given value and chain position @@ -257,7 +268,7 @@ pub fn receive_output_to_address( wallet: &mut Wallet, addr: Address, value: u64, - pos: ChainPosition, + receive_to: impl Into, ) -> OutPoint { let tx = Transaction { version: transaction::Version::ONE, @@ -272,15 +283,9 @@ pub fn receive_output_to_address( let txid = tx.compute_txid(); insert_tx(wallet, tx); - match pos { - ChainPosition::Confirmed { anchor, .. } => { - insert_anchor(wallet, txid, anchor); - } - ChainPosition::Unconfirmed { last_seen } => { - if let Some(last_seen) = last_seen { - insert_seen_at(wallet, txid, last_seen); - } - } + match receive_to.into() { + ReceiveTo::Block(anchor) => insert_anchor(wallet, txid, anchor), + ReceiveTo::Mempool(last_seen) => insert_seen_at(wallet, txid, last_seen), } OutPoint { txid, vout: 0 } diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs index e2e905020..9174ebab2 100644 --- a/crates/wallet/src/wallet/mod.rs +++ b/crates/wallet/src/wallet/mod.rs @@ -1063,11 +1063,9 @@ impl Wallet { let graph = self.indexed_graph.graph(); Some(WalletTx { - chain_position: graph.get_chain_position( - &self.chain, - self.chain.tip().block_id(), - txid, - )?, + chain_position: graph + .get_chain_position(&self.chain, self.chain.tip().block_id(), txid)? + .cloned(), tx_node: graph.get_tx_node(txid)?, }) } diff --git a/crates/wallet/tests/wallet.rs b/crates/wallet/tests/wallet.rs index 4a53c350f..4afb66e65 100644 --- a/crates/wallet/tests/wallet.rs +++ b/crates/wallet/tests/wallet.rs @@ -1223,7 +1223,7 @@ fn test_create_tx_add_utxo() { let txid = small_output_tx.compute_txid(); insert_tx(&mut wallet, small_output_tx); let anchor = ConfirmationBlockTime { - block_id: wallet.latest_checkpoint().block_id(), + block_id: wallet.latest_checkpoint().get(2000).unwrap().block_id(), confirmation_time: 200, }; insert_anchor(&mut wallet, txid, anchor); @@ -1270,7 +1270,7 @@ fn test_create_tx_manually_selected_insufficient() { let txid = small_output_tx.compute_txid(); insert_tx(&mut wallet, small_output_tx.clone()); let anchor = ConfirmationBlockTime { - block_id: wallet.latest_checkpoint().block_id(), + block_id: wallet.latest_checkpoint().get(2000).unwrap().block_id(), confirmation_time: 200, }; insert_anchor(&mut wallet, txid, anchor); @@ -1496,11 +1496,7 @@ fn test_create_tx_increment_change_index() { .create_wallet_no_persist() .unwrap(); // fund wallet - receive_output( - &mut wallet, - amount, - ChainPosition::Unconfirmed { last_seen: Some(0) }, - ); + receive_output(&mut wallet, amount, ReceiveTo::Mempool(0)); // create tx let mut builder = wallet.build_tx(); builder.add_recipient(recipient.clone(), Amount::from_sat(test.to_send)); @@ -2164,12 +2160,15 @@ fn test_bump_fee_remove_output_manually_selected_only() { }], }; + let position: ChainPosition = + wallet.transactions().last().unwrap().chain_position; insert_tx(&mut wallet, init_tx.clone()); - let anchor = ConfirmationBlockTime { - block_id: wallet.latest_checkpoint().block_id(), - confirmation_time: 200, - }; - insert_anchor(&mut wallet, init_tx.compute_txid(), anchor); + match position { + ChainPosition::Confirmed { anchor, .. } => { + insert_anchor(&mut wallet, init_tx.compute_txid(), anchor) + } + other => panic!("all wallet txs must be confirmed: {:?}", other), + } let outpoint = OutPoint { txid: init_tx.compute_txid(), @@ -2213,12 +2212,13 @@ fn test_bump_fee_add_input() { }], }; let txid = init_tx.compute_txid(); + let pos: ChainPosition = + wallet.transactions().last().unwrap().chain_position; insert_tx(&mut wallet, init_tx); - let anchor = ConfirmationBlockTime { - block_id: wallet.latest_checkpoint().block_id(), - confirmation_time: 200, - }; - insert_anchor(&mut wallet, txid, anchor); + match pos { + ChainPosition::Confirmed { anchor, .. } => insert_anchor(&mut wallet, txid, anchor), + other => panic!("all wallet txs must be confirmed: {:?}", other), + } let addr = Address::from_str("2N1Ffz3WaNzbeLFBb51xyFMHYSEUXcbiSoX") .unwrap() @@ -2605,11 +2605,7 @@ fn test_bump_fee_unconfirmed_inputs_only() { let psbt = builder.finish().unwrap(); // Now we receive one transaction with 0 confirmations. We won't be able to use that for // fee bumping, as it's still unconfirmed! - receive_output( - &mut wallet, - 25_000, - ChainPosition::Unconfirmed { last_seen: Some(0) }, - ); + receive_output(&mut wallet, 25_000, ReceiveTo::Mempool(0)); let mut tx = psbt.extract_tx().expect("failed to extract tx"); let txid = tx.compute_txid(); for txin in &mut tx.input { @@ -2634,11 +2630,7 @@ fn test_bump_fee_unconfirmed_input() { .assume_checked(); // We receive a tx with 0 confirmations, which will be used as an input // in the drain tx. - receive_output( - &mut wallet, - 25_000, - ChainPosition::Unconfirmed { last_seen: Some(0) }, - ); + receive_output(&mut wallet, 25_000, ReceiveTo::Mempool(0)); let mut builder = wallet.build_tx(); builder.drain_wallet().drain_to(addr.script_pubkey()); let psbt = builder.finish().unwrap(); @@ -3048,7 +3040,7 @@ fn test_next_unused_address() { assert_eq!(next_unused_addr.index, 0); // use the above address - receive_output_in_latest_block(&mut wallet, 25_000); + receive_output(&mut wallet, 25_000, ReceiveTo::Mempool(0)); assert_eq!( wallet @@ -4108,17 +4100,14 @@ fn test_keychains_with_overlapping_spks() { .last() .unwrap() .address; - let chain_position = ChainPosition::Confirmed { - anchor: ConfirmationBlockTime { - block_id: BlockId { - height: 2000, - hash: BlockHash::all_zeros(), - }, - confirmation_time: 0, + let anchor = ConfirmationBlockTime { + block_id: BlockId { + height: 2000, + hash: BlockHash::all_zeros(), }, - transitively: None, + confirmation_time: 0, }; - let _outpoint = receive_output_to_address(&mut wallet, addr, 8000, chain_position); + let _outpoint = receive_output_to_address(&mut wallet, addr, 8000, anchor); assert_eq!(wallet.balance().confirmed, Amount::from_sat(58000)); } @@ -4207,11 +4196,7 @@ fn single_descriptor_wallet_can_create_tx_and_receive_change() { .unwrap(); assert_eq!(wallet.keychains().count(), 1); let amt = Amount::from_sat(5_000); - receive_output( - &mut wallet, - 2 * amt.to_sat(), - ChainPosition::Unconfirmed { last_seen: Some(2) }, - ); + receive_output(&mut wallet, 2 * amt.to_sat(), ReceiveTo::Mempool(2)); // create spend tx that produces a change output let addr = Address::from_str("bcrt1qc6fweuf4xjvz4x3gx3t9e0fh4hvqyu2qw4wvxm") .unwrap() @@ -4237,11 +4222,7 @@ fn single_descriptor_wallet_can_create_tx_and_receive_change() { #[test] fn test_transactions_sort_by() { let (mut wallet, _txid) = get_funded_wallet_wpkh(); - receive_output( - &mut wallet, - 25_000, - ChainPosition::Unconfirmed { last_seen: Some(0) }, - ); + receive_output(&mut wallet, 25_000, ReceiveTo::Mempool(0)); // sort by chain position, unconfirmed then confirmed by descending block height let sorted_txs: Vec = diff --git a/example-crates/example_cli/src/lib.rs b/example-crates/example_cli/src/lib.rs index 6a97252fc..8c17348b3 100644 --- a/example-crates/example_cli/src/lib.rs +++ b/example-crates/example_cli/src/lib.rs @@ -421,12 +421,8 @@ pub fn planned_utxos( let outpoints = graph.index.outpoints(); graph .graph() - .try_filter_chain_unspents(chain, chain_tip, outpoints.iter().cloned()) - .filter_map(|r| -> Option> { - let (k, i, full_txo) = match r { - Err(err) => return Some(Err(err)), - Ok(((k, i), full_txo)) => (k, i, full_txo), - }; + .try_filter_chain_unspents(chain, chain_tip, outpoints.iter().cloned())? + .filter_map(|((k, i), full_txo)| -> Option> { let desc = graph .index .keychains() @@ -560,26 +556,18 @@ pub fn handle_commands( } => { let txouts = graph .graph() - .try_filter_chain_txouts(chain, chain_tip, outpoints.iter().cloned()) - .filter(|r| match r { - Ok((_, full_txo)) => match (spent, unspent) { - (true, false) => full_txo.spent_by.is_some(), - (false, true) => full_txo.spent_by.is_none(), - _ => true, - }, - // always keep errored items - Err(_) => true, + .try_filter_chain_txouts(chain, chain_tip, outpoints.iter().cloned())? + .filter(|(_, full_txo)| match (spent, unspent) { + (true, false) => full_txo.spent_by.is_some(), + (false, true) => full_txo.spent_by.is_none(), + _ => true, }) - .filter(|r| match r { - Ok((_, full_txo)) => match (confirmed, unconfirmed) { - (true, false) => full_txo.chain_position.is_confirmed(), - (false, true) => !full_txo.chain_position.is_confirmed(), - _ => true, - }, - // always keep errored items - Err(_) => true, + .filter(|(_, full_txo)| match (confirmed, unconfirmed) { + (true, false) => full_txo.chain_position.is_confirmed(), + (false, true) => !full_txo.chain_position.is_confirmed(), + _ => true, }) - .collect::, _>>()?; + .collect::>(); for (spk_i, full_txo) in txouts { let addr = Address::from_script(&full_txo.txout.script_pubkey, network)?; From 8fbee12b1c24ec0004fda40eaf9330a87a7f4161 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 19 Nov 2024 17:26:50 +1100 Subject: [PATCH 04/16] feat(chain)!: rm `get_chain_position` and associated methods --- crates/chain/src/tx_graph.rs | 222 +------------------- crates/chain/tests/test_indexed_tx_graph.rs | 21 +- crates/chain/tests/test_tx_graph.rs | 180 ++++++++-------- crates/wallet/src/wallet/mod.rs | 88 ++++---- 4 files changed, 162 insertions(+), 349 deletions(-) diff --git a/crates/chain/src/tx_graph.rs b/crates/chain/src/tx_graph.rs index 0e7dd33fa..71fde7188 100644 --- a/crates/chain/src/tx_graph.rs +++ b/crates/chain/src/tx_graph.rs @@ -20,8 +20,7 @@ //! identifying and traversing conflicts and descendants of a given transaction. Some [`TxGraph`] //! methods only consider transactions that are "canonical" (i.e., in the best chain or in mempool). //! We decide which transactions are canonical based on the transaction's anchors and the -//! `last_seen` (as unconfirmed) timestamp; see the [`try_get_chain_position`] documentation for -//! more details. +//! `last_seen` (as unconfirmed) timestamp. //! //! The [`ChangeSet`] reports changes made to a [`TxGraph`]; it can be used to either save to //! persistent storage, or to be applied to another [`TxGraph`]. @@ -89,7 +88,6 @@ //! let changeset = graph.apply_update(update); //! assert!(changeset.is_empty()); //! ``` -//! [`try_get_chain_position`]: TxGraph::try_get_chain_position //! [`insert_txout`]: TxGraph::insert_txout use crate::collections::*; @@ -791,224 +789,6 @@ impl TxGraph { } impl TxGraph { - /// Get the position of the transaction in `chain` with tip `chain_tip`. - /// - /// Chain data is fetched from `chain`, a [`ChainOracle`] implementation. - /// - /// This method returns `Ok(None)` if the transaction is not found in the chain, and no longer - /// belongs in the mempool. The following factors are used to approximate whether an - /// unconfirmed transaction exists in the mempool (not evicted): - /// - /// 1. Unconfirmed transactions that conflict with confirmed transactions are evicted. - /// 2. Unconfirmed transactions that spend from transactions that are evicted, are also - /// evicted. - /// 3. Given two conflicting unconfirmed transactions, the transaction with the lower - /// `last_seen_unconfirmed` parameter is evicted. A transaction's `last_seen_unconfirmed` - /// parameter is the max of all it's descendants' `last_seen_unconfirmed` parameters. If the - /// final `last_seen_unconfirmed`s are the same, the transaction with the lower `txid` (by - /// lexicographical order) is evicted. - /// - /// # Error - /// - /// An error will occur if the [`ChainOracle`] implementation (`chain`) fails. If the - /// [`ChainOracle`] is infallible, [`get_chain_position`] can be used instead. - /// - /// [`get_chain_position`]: Self::get_chain_position - pub fn try_get_chain_position( - &self, - chain: &C, - chain_tip: BlockId, - txid: Txid, - ) -> Result>, C::Error> { - let tx_node = match self.txs.get(&txid) { - Some(v) => v, - None => return Ok(None), - }; - - for anchor in self.anchors.get(&txid).unwrap_or(&self.empty_anchors) { - match chain.is_block_in_chain(anchor.anchor_block(), chain_tip)? { - Some(true) => { - return Ok(Some(ChainPosition::Confirmed { - anchor, - transitively: None, - })) - } - _ => continue, - } - } - - // If no anchors are in best chain and we don't have a last_seen, we can return - // early because by definition the tx doesn't have a chain position. - let last_seen = match self.last_seen.get(&txid) { - Some(t) => *t, - None => return Ok(None), - }; - - // The tx is not anchored to a block in the best chain, which means that it - // might be in mempool, or it might have been dropped already. - // Let's check conflicts to find out! - let tx = match tx_node { - TxNodeInternal::Whole(tx) => { - // A coinbase tx that is not anchored in the best chain cannot be unconfirmed and - // should always be filtered out. - if tx.is_coinbase() { - return Ok(None); - } - tx.clone() - } - TxNodeInternal::Partial(_) => { - // Partial transactions (outputs only) cannot have conflicts. - return Ok(None); - } - }; - - // We want to retrieve all the transactions that conflict with us, plus all the - // transactions that conflict with our unconfirmed ancestors, since they conflict with us - // as well. - // We only traverse unconfirmed ancestors since conflicts of confirmed transactions - // cannot be in the best chain. - - // First of all, we retrieve all our ancestors. Since we're using `new_include_root`, the - // resulting array will also include `tx` - let unconfirmed_ancestor_txs = - TxAncestors::new_include_root(self, tx.clone(), |_, ancestor_tx: Arc| { - let tx_node = self.get_tx_node(ancestor_tx.as_ref().compute_txid())?; - // We're filtering the ancestors to keep only the unconfirmed ones (= no anchors in - // the best chain) - for block in tx_node.anchors { - match chain.is_block_in_chain(block.anchor_block(), chain_tip) { - Ok(Some(true)) => return None, - Err(e) => return Some(Err(e)), - _ => continue, - } - } - Some(Ok(tx_node)) - }) - .collect::, C::Error>>()?; - - // We determine our tx's last seen, which is the max between our last seen, - // and our unconf descendants' last seen. - let unconfirmed_descendants_txs = TxDescendants::new_include_root( - self, - tx.as_ref().compute_txid(), - |_, descendant_txid: Txid| { - let tx_node = self.get_tx_node(descendant_txid)?; - // We're filtering the ancestors to keep only the unconfirmed ones (= no anchors in - // the best chain) - for block in tx_node.anchors { - match chain.is_block_in_chain(block.anchor_block(), chain_tip) { - Ok(Some(true)) => return None, - Err(e) => return Some(Err(e)), - _ => continue, - } - } - Some(Ok(tx_node)) - }, - ) - .collect::, C::Error>>()?; - - let tx_last_seen = unconfirmed_descendants_txs - .iter() - .max_by_key(|tx| tx.last_seen_unconfirmed) - .map(|tx| tx.last_seen_unconfirmed) - .expect("descendants always includes at least one transaction (the root tx"); - - // Now we traverse our ancestors and consider all their conflicts - for tx_node in unconfirmed_ancestor_txs { - // We retrieve all the transactions conflicting with this specific ancestor - let conflicting_txs = - self.walk_conflicts(tx_node.tx.as_ref(), |_, txid| self.get_tx_node(txid)); - - // If a conflicting tx is in the best chain, or has `last_seen` higher than this ancestor, then - // this tx cannot exist in the best chain - for conflicting_tx in conflicting_txs { - for block in conflicting_tx.anchors { - if chain.is_block_in_chain(block.anchor_block(), chain_tip)? == Some(true) { - return Ok(None); - } - } - if conflicting_tx.last_seen_unconfirmed > tx_last_seen { - return Ok(None); - } - if conflicting_tx.last_seen_unconfirmed == Some(last_seen) - && conflicting_tx.as_ref().compute_txid() > tx.as_ref().compute_txid() - { - // Conflicting tx has priority if txid of conflicting tx > txid of original tx - return Ok(None); - } - } - } - - Ok(Some(ChainPosition::Unconfirmed { - last_seen: Some(last_seen), - })) - } - - /// Get the position of the transaction in `chain` with tip `chain_tip`. - /// - /// This is the infallible version of [`try_get_chain_position`]. - /// - /// [`try_get_chain_position`]: Self::try_get_chain_position - pub fn get_chain_position>( - &self, - chain: &C, - chain_tip: BlockId, - txid: Txid, - ) -> Option> { - self.try_get_chain_position(chain, chain_tip, txid) - .expect("error is infallible") - } - - /// Get the txid of the spending transaction and where the spending transaction is observed in - /// the `chain` of `chain_tip`. - /// - /// If no in-chain transaction spends `outpoint`, `None` will be returned. - /// - /// # Error - /// - /// An error will occur only if the [`ChainOracle`] implementation (`chain`) fails. - /// - /// If the [`ChainOracle`] is infallible, [`get_chain_spend`] can be used instead. - /// - /// [`get_chain_spend`]: Self::get_chain_spend - pub fn try_get_chain_spend( - &self, - chain: &C, - chain_tip: BlockId, - outpoint: OutPoint, - ) -> Result, Txid)>, C::Error> { - if self - .try_get_chain_position(chain, chain_tip, outpoint.txid)? - .is_none() - { - return Ok(None); - } - if let Some(spends) = self.spends.get(&outpoint) { - for &txid in spends { - if let Some(observed_at) = self.try_get_chain_position(chain, chain_tip, txid)? { - return Ok(Some((observed_at, txid))); - } - } - } - Ok(None) - } - - /// Get the txid of the spending transaction and where the spending transaction is observed in - /// the `chain` of `chain_tip`. - /// - /// This is the infallible version of [`try_get_chain_spend`] - /// - /// [`try_get_chain_spend`]: Self::try_get_chain_spend - pub fn get_chain_spend>( - &self, - chain: &C, - static_block: BlockId, - outpoint: OutPoint, - ) -> Option<(ChainPosition<&A>, Txid)> { - self.try_get_chain_spend(chain, static_block, outpoint) - .expect("error is infallible") - } - /// List graph transactions that are in `chain` with `chain_tip`. /// /// Each transaction is represented as a [`CanonicalTx`] that contains where the transaction is diff --git a/crates/chain/tests/test_indexed_tx_graph.rs b/crates/chain/tests/test_indexed_tx_graph.rs index e37a639c2..882041fba 100644 --- a/crates/chain/tests/test_indexed_tx_graph.rs +++ b/crates/chain/tests/test_indexed_tx_graph.rs @@ -588,14 +588,17 @@ fn test_get_chain_position() { } // check chain position - let res = graph + let chain_pos = graph .graph() - .get_chain_position(chain, chain.tip().block_id(), txid); - assert_eq!( - res.map(ChainPosition::cloned), - exp_pos, - "failed test case: {name}" - ); + .list_canonical_txs(chain, chain.tip().block_id()) + .find_map(|canon_tx| { + if canon_tx.tx_node.txid == txid { + Some(canon_tx.chain_position) + } else { + None + } + }); + assert_eq!(chain_pos, exp_pos, "failed test case: {name}"); } [ @@ -655,7 +658,7 @@ fn test_get_chain_position() { exp_pos: Some(ChainPosition::Unconfirmed { last_seen: Some(2) }), }, TestCase { - name: "tx unknown anchor - no chain pos", + name: "tx unknown anchor - unconfirmed", tx: Transaction { output: vec![TxOut { value: Amount::ONE_BTC, @@ -665,7 +668,7 @@ fn test_get_chain_position() { }, anchor: Some(block_id!(2, "B'")), last_seen: None, - exp_pos: None, + exp_pos: Some(ChainPosition::Unconfirmed { last_seen: None }), }, ] .into_iter() diff --git a/crates/chain/tests/test_tx_graph.rs b/crates/chain/tests/test_tx_graph.rs index 8f1634ba1..ef57ac15b 100644 --- a/crates/chain/tests/test_tx_graph.rs +++ b/crates/chain/tests/test_tx_graph.rs @@ -877,63 +877,77 @@ fn test_chain_spends() { ); } - // Assert that confirmed spends are returned correctly. - assert_eq!( - graph.get_chain_spend( - &local_chain, - tip.block_id(), - OutPoint::new(tx_0.compute_txid(), 0) - ), - Some(( - ChainPosition::Confirmed { - anchor: &ConfirmationBlockTime { - block_id: BlockId { - hash: tip.get(98).unwrap().hash(), - height: 98, + let build_canonical_spends = + |chain: &LocalChain, tx_graph: &TxGraph| -> HashMap { + tx_graph + .filter_chain_txouts( + chain, + tip.block_id(), + tx_graph.all_txouts().map(|(op, _)| ((), op)), + ) + .filter_map(|(_, full_txo)| Some((full_txo.outpoint, full_txo.spent_by?))) + .collect() + }; + let build_canonical_positions = |chain: &LocalChain, + tx_graph: &TxGraph| + -> HashMap> { + tx_graph + .list_canonical_txs(chain, tip.block_id()) + .map(|canon_tx| (canon_tx.tx_node.txid, canon_tx.chain_position)) + .collect() + }; + + { + let canonical_spends = build_canonical_spends(&local_chain, &graph); + let canonical_positions = build_canonical_positions(&local_chain, &graph); + + // Assert that confirmed spends are returned correctly. + assert_eq!( + canonical_spends + .get(&OutPoint::new(tx_0.compute_txid(), 0)) + .cloned(), + Some(( + ChainPosition::Confirmed { + anchor: ConfirmationBlockTime { + block_id: tip.get(98).unwrap().block_id(), + confirmation_time: 100 }, + transitively: None, + }, + tx_1.compute_txid(), + )), + ); + // Check if chain position is returned correctly. + assert_eq!( + canonical_positions.get(&tx_0.compute_txid()).cloned(), + Some(ChainPosition::Confirmed { + anchor: ConfirmationBlockTime { + block_id: tip.get(95).unwrap().block_id(), confirmation_time: 100 }, transitively: None - }, - tx_1.compute_txid(), - )), - ); - - // Check if chain position is returned correctly. - assert_eq!( - graph.get_chain_position(&local_chain, tip.block_id(), tx_0.compute_txid()), - // Some(ObservedAs::Confirmed(&local_chain.get_block(95).expect("block expected"))), - Some(ChainPosition::Confirmed { - anchor: &ConfirmationBlockTime { - block_id: BlockId { - hash: tip.get(95).unwrap().hash(), - height: 95, - }, - confirmation_time: 100 - }, - transitively: None - }) - ); + }) + ); + } // Mark the unconfirmed as seen and check correct ObservedAs status is returned. let _ = graph.insert_seen_at(tx_2.compute_txid(), 1234567); + { + let canonical_spends = build_canonical_spends(&local_chain, &graph); - // Check chain spend returned correctly. - assert_eq!( - graph - .get_chain_spend( - &local_chain, - tip.block_id(), - OutPoint::new(tx_0.compute_txid(), 1) - ) - .unwrap(), - ( - ChainPosition::Unconfirmed { - last_seen: Some(1234567) - }, - tx_2.compute_txid() - ) - ); + // Check chain spend returned correctly. + assert_eq!( + canonical_spends + .get(&OutPoint::new(tx_0.compute_txid(), 1)) + .cloned(), + Some(( + ChainPosition::Unconfirmed { + last_seen: Some(1234567) + }, + tx_2.compute_txid() + )) + ); + } // A conflicting transaction that conflicts with tx_1. let tx_1_conflict = Transaction { @@ -944,11 +958,14 @@ fn test_chain_spends() { ..new_tx(0) }; let _ = graph.insert_tx(tx_1_conflict.clone()); + { + let canonical_positions = build_canonical_positions(&local_chain, &graph); - // Because this tx conflicts with an already confirmed transaction, chain position should return none. - assert!(graph - .get_chain_position(&local_chain, tip.block_id(), tx_1_conflict.compute_txid()) - .is_none()); + // Because this tx conflicts with an already confirmed transaction, chain position should return none. + assert!(canonical_positions + .get(&tx_1_conflict.compute_txid()) + .is_none()); + } // Another conflicting tx that conflicts with tx_2. let tx_2_conflict = Transaction { @@ -958,42 +975,39 @@ fn test_chain_spends() { }], ..new_tx(0) }; - // Insert in graph and mark it as seen. let _ = graph.insert_tx(tx_2_conflict.clone()); let _ = graph.insert_seen_at(tx_2_conflict.compute_txid(), 1234568); + { + let canonical_spends = build_canonical_spends(&local_chain, &graph); + let canonical_positions = build_canonical_positions(&local_chain, &graph); - // This should return a valid observation with correct last seen. - assert_eq!( - graph - .get_chain_position(&local_chain, tip.block_id(), tx_2_conflict.compute_txid()) - .expect("position expected"), - ChainPosition::Unconfirmed { - last_seen: Some(1234568) - } - ); - - // Chain_spend now catches the new transaction as the spend. - assert_eq!( - graph - .get_chain_spend( - &local_chain, - tip.block_id(), - OutPoint::new(tx_0.compute_txid(), 1) - ) - .expect("expect observation"), - ( - ChainPosition::Unconfirmed { + // This should return a valid observation with correct last seen. + assert_eq!( + canonical_positions + .get(&tx_2_conflict.compute_txid()) + .cloned(), + Some(ChainPosition::Unconfirmed { last_seen: Some(1234568) - }, - tx_2_conflict.compute_txid() - ) - ); + }) + ); - // Chain position of the `tx_2` is now none, as it is older than `tx_2_conflict` - assert!(graph - .get_chain_position(&local_chain, tip.block_id(), tx_2.compute_txid()) - .is_none()); + // Chain_spend now catches the new transaction as the spend. + assert_eq!( + canonical_spends + .get(&OutPoint::new(tx_0.compute_txid(), 1)) + .cloned(), + Some(( + ChainPosition::Unconfirmed { + last_seen: Some(1234568) + }, + tx_2_conflict.compute_txid() + )) + ); + + // Chain position of the `tx_2` is now none, as it is older than `tx_2_conflict` + assert!(canonical_positions.get(&tx_2.compute_txid()).is_none()); + } } /// Ensure that `last_seen` values only increase during [`Merge::merge`]. diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs index 9174ebab2..542db3b06 100644 --- a/crates/wallet/src/wallet/mod.rs +++ b/crates/wallet/src/wallet/mod.rs @@ -59,7 +59,7 @@ pub mod signer; pub mod tx_builder; pub(crate) mod utils; -use crate::collections::{BTreeMap, HashMap}; +use crate::collections::{BTreeMap, HashMap, HashSet}; use crate::descriptor::{ check_wallet_descriptor, error::Error as DescriptorError, policy::BuildSatisfaction, DerivedDescriptor, DescriptorMeta, ExtendedDescriptor, ExtractPolicy, IntoWalletDescriptor, @@ -1061,13 +1061,9 @@ impl Wallet { /// [`Anchor`]: bdk_chain::Anchor pub fn get_tx(&self, txid: Txid) -> Option { let graph = self.indexed_graph.graph(); - - Some(WalletTx { - chain_position: graph - .get_chain_position(&self.chain, self.chain.tip().block_id(), txid)? - .cloned(), - tx_node: graph.get_tx_node(txid)?, - }) + graph + .list_canonical_txs(&self.chain, self.chain.tip().block_id()) + .find(|tx| tx.tx_node.txid == txid) } /// Iterate over the transactions in the wallet. @@ -1588,6 +1584,10 @@ impl Wallet { let graph = self.indexed_graph.graph(); let txout_index = &self.indexed_graph.index; let chain_tip = self.chain.tip().block_id(); + let chain_positions = graph + .list_canonical_txs(&self.chain, chain_tip) + .map(|canon_tx| (canon_tx.tx_node.txid, canon_tx.chain_position)) + .collect::>(); let mut tx = graph .get_tx(txid) @@ -1595,10 +1595,11 @@ impl Wallet { .as_ref() .clone(); - let pos = graph - .get_chain_position(&self.chain, chain_tip, txid) - .ok_or(BuildFeeBumpError::TransactionNotFound(txid))?; - if pos.is_confirmed() { + if chain_positions + .get(&txid) + .ok_or(BuildFeeBumpError::TransactionNotFound(txid))? + .is_confirmed() + { return Err(BuildFeeBumpError::TransactionConfirmed(txid)); } @@ -1629,10 +1630,10 @@ impl Wallet { .ok_or(BuildFeeBumpError::UnknownUtxo(txin.previous_output))?; let txout = &prev_tx.output[txin.previous_output.vout as usize]; - let chain_position = graph - .get_chain_position(&self.chain, chain_tip, txin.previous_output.txid) - .ok_or(BuildFeeBumpError::UnknownUtxo(txin.previous_output))? - .cloned(); + let chain_position = chain_positions + .get(&txin.previous_output.txid) + .cloned() + .ok_or(BuildFeeBumpError::UnknownUtxo(txin.previous_output))?; let weighted_utxo = match txout_index.index_of_spk(txout.script_pubkey.clone()) { Some(&(keychain, derivation_index)) => { @@ -1831,9 +1832,28 @@ impl Wallet { psbt: &mut Psbt, sign_options: SignOptions, ) -> Result { + let tx = &psbt.unsigned_tx; let chain_tip = self.chain.tip().block_id(); + let prev_txids = tx + .input + .iter() + .map(|txin| txin.previous_output.txid) + .collect::>(); + let confirmation_heights = self + .indexed_graph + .graph() + .list_canonical_txs(&self.chain, chain_tip) + .filter(|canon_tx| prev_txids.contains(&canon_tx.tx_node.txid)) + .take(prev_txids.len()) + .map(|canon_tx| { + let txid = canon_tx.tx_node.txid; + match canon_tx.chain_position { + ChainPosition::Confirmed { anchor, .. } => (txid, anchor.block_id.height), + ChainPosition::Unconfirmed { .. } => (txid, u32::MAX), + } + }) + .collect::>(); - let tx = &psbt.unsigned_tx; let mut finished = true; for (n, input) in tx.input.iter().enumerate() { @@ -1844,15 +1864,9 @@ impl Wallet { if psbt_input.final_script_sig.is_some() || psbt_input.final_script_witness.is_some() { continue; } - let confirmation_height = self - .indexed_graph - .graph() - .get_chain_position(&self.chain, chain_tip, input.previous_output.txid) - .map(|chain_position| { - chain_position - .confirmation_height_upper_bound() - .unwrap_or(u32::MAX) - }); + let confirmation_height = confirmation_heights + .get(&input.previous_output.txid) + .copied(); let current_height = sign_options .assume_height .unwrap_or_else(|| self.chain.tip().height()); @@ -2012,20 +2026,22 @@ impl Wallet { return (must_spend, vec![]); } + let canon_txs = self + .indexed_graph + .graph() + .list_canonical_txs(&self.chain, chain_tip) + .map(|canon_tx| (canon_tx.tx_node.txid, canon_tx)) + .collect::>(); + let satisfies_confirmed = may_spend .iter() .map(|u| -> bool { let txid = u.0.outpoint.txid; - let tx = match self.indexed_graph.graph().get_tx(txid) { - Some(tx) => tx, - None => return false, - }; - let chain_position = match self.indexed_graph.graph().get_chain_position( - &self.chain, - chain_tip, - txid, - ) { - Some(chain_position) => chain_position.cloned(), + let (chain_position, tx) = match canon_txs.get(&txid) { + Some(CanonicalTx { + chain_position, + tx_node, + }) => (chain_position, tx_node.tx.clone()), None => return false, }; From 4325e2c25e037129c7878f347dd32cc48b771bb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 26 Nov 2024 15:10:18 +1100 Subject: [PATCH 05/16] test(chain): Add transitive anchor tests --- crates/chain/tests/test_tx_graph_conflicts.rs | 77 ++++++++++++++++++- 1 file changed, 74 insertions(+), 3 deletions(-) diff --git a/crates/chain/tests/test_tx_graph_conflicts.rs b/crates/chain/tests/test_tx_graph_conflicts.rs index 2e64e3912..a2b6abdc3 100644 --- a/crates/chain/tests/test_tx_graph_conflicts.rs +++ b/crates/chain/tests/test_tx_graph_conflicts.rs @@ -81,10 +81,8 @@ fn test_tx_conflict_handling() { exp_chain_txouts: HashSet::from([("confirmed_genesis", 0), ("confirmed_conflict", 0)]), exp_unspents: HashSet::from([("confirmed_conflict", 0)]), exp_balance: Balance { - immature: Amount::ZERO, - trusted_pending: Amount::ZERO, - untrusted_pending: Amount::ZERO, confirmed: Amount::from_sat(20000), + ..Default::default() }, }, Scenario { @@ -593,6 +591,79 @@ fn test_tx_conflict_handling() { confirmed: Amount::from_sat(50000), }, }, + Scenario { + name: "transitively confirmed ancestors", + tx_templates: &[ + TxTemplate { + tx_name: "first", + inputs: &[TxInTemplate::Bogus], + outputs: &[TxOutTemplate::new(1000, Some(0))], + ..Default::default() + }, + TxTemplate { + tx_name: "second", + inputs: &[TxInTemplate::PrevTx("first", 0)], + outputs: &[TxOutTemplate::new(900, Some(0))], + ..Default::default() + }, + TxTemplate { + tx_name: "anchored", + inputs: &[TxInTemplate::PrevTx("second", 0)], + outputs: &[TxOutTemplate::new(800, Some(0))], + anchors: &[block_id!(3, "D")], + ..Default::default() + }, + ], + exp_chain_txs: HashSet::from(["first", "second", "anchored"]), + exp_chain_txouts: HashSet::from([("first", 0), ("second", 0), ("anchored", 0)]), + exp_unspents: HashSet::from([("anchored", 0)]), + exp_balance: Balance { + immature: Amount::ZERO, + trusted_pending: Amount::ZERO, + untrusted_pending: Amount::ZERO, + confirmed: Amount::from_sat(800), + } + }, + Scenario { + name: "transitively anchored txs should have priority over last seen", + tx_templates: &[ + TxTemplate { + tx_name: "root", + inputs: &[TxInTemplate::Bogus], + outputs: &[TxOutTemplate::new(10_000, Some(0))], + anchors: &[block_id!(1, "B")], + ..Default::default() + }, + TxTemplate { + tx_name: "last_seen_conflict", + inputs: &[TxInTemplate::PrevTx("root", 0)], + outputs: &[TxOutTemplate::new(9900, Some(1))], + last_seen: Some(1000), + ..Default::default() + }, + TxTemplate { + tx_name: "transitively_anchored_conflict", + inputs: &[TxInTemplate::PrevTx("root", 0)], + outputs: &[TxOutTemplate::new(9000, Some(1))], + last_seen: Some(100), + ..Default::default() + }, + TxTemplate { + tx_name: "anchored", + inputs: &[TxInTemplate::PrevTx("transitively_anchored_conflict", 0)], + outputs: &[TxOutTemplate::new(8000, Some(2))], + anchors: &[block_id!(4, "E")], + ..Default::default() + }, + ], + exp_chain_txs: HashSet::from(["root", "transitively_anchored_conflict", "anchored"]), + exp_chain_txouts: HashSet::from([("root", 0), ("transitively_anchored_conflict", 0), ("anchored", 0)]), + exp_unspents: HashSet::from([("anchored", 0)]), + exp_balance: Balance { + confirmed: Amount::from_sat(8000), + ..Default::default() + } + } ]; for scenario in scenarios { From e9854455ca77875a6ff79047726064ba42f94f29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Sun, 1 Dec 2024 15:59:09 +1100 Subject: [PATCH 06/16] docs: Add ADR for `O(n)` canonicalization algorithm --- docs/adr/0003_canonicalization_algorithm.md | 83 +++++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 docs/adr/0003_canonicalization_algorithm.md diff --git a/docs/adr/0003_canonicalization_algorithm.md b/docs/adr/0003_canonicalization_algorithm.md new file mode 100644 index 000000000..503e857e6 --- /dev/null +++ b/docs/adr/0003_canonicalization_algorithm.md @@ -0,0 +1,83 @@ +# Introduce `O(n)` Canonicalization Algorithm + +* Status: Proposed +* Authors: @LLFourn, @evanlinjin +* Date: 2024-12-01 +* Targeted modules: `bdk_chain` +* Associated Tickets/PRs: Issue #1665, ~PR #1659~, PR #1670 + +## Context and Problem Statement + +The [2024 Wizardsardine BDK code audit](https://gist.github.com/darosior/4aeb9512d7f1ac7666abc317d6f9453b) uncovered the severity of the performance issues in the original canonicalization logic. The problem is especially severe for wallet histories with many unconfirmed and conflicting transactions. This can be a dDos vector if BDK is used in server-side applications. The time complexity of the original canonicalization logic is $O(n^2)$. + +The old canonicalization logic is based on `TxGraph::get_chain_position`. This is called on every transaction included in `TxGraph`, having to traverse backwards and forwards to check that all ancestors do not conflict with anything that is anchored in the best chain and that no conflict has a higher `last-seen` value. Also note that `last-seen` values are transitive, so to determine the *actual* `last-seen` value, we need to iterate through all descendants. + +## Considered Options + +#### Option 1: Introduce a `canonical_cache` as a parameter to all `get_chain_position`-based methods. + +The `canonical_cache` will include both `canonical` and `not_canonical` sets of txids. This avoids revisiting what has already been visited. + +**Pros:** +* Least API and code changes. + +**Cons:** +* The API can be misused. Can get wildly wrong results if the `canonical_cache` parameter is used across updates to `TxGraph` or the `ChainOracle` impl. +* Visiting transactions in a certain order may decrease the number of traversals. I.e. if we call `get_chain_position` on transactions with anchors first, `get_chain_position` calls on non-anchored transactions later on won't need to do as much work. Visiting order is not enforced if we stick to a `get_chain_position`-based API. + +#### Option 2: Traverse `TxGraph` spends forwards, starting from graph roots. + +For this algorithm, we maintain two `txid` sets; `maybe_canonical` and `not_canonical`. Note that these sets are not mutually exclusive since we are traversing spends, and a transaction can have multiple inputs (spends). When we arrive at a transaction's input (spend), we may not have checked all of the transaction's other inputs to be sure that an ancestor does not conflict with a transaction that is anchored or has a higher last-seen value. + +**Pros:** +* API cannot be misused (as it can in option 1). +* We can traverse transactions in a pseudo-chronological order. + +**Cons:** +* Duplicate work may have to be done if we have transactions with multiple inputs. We may mark a subset of transactions as `maybe_canonical`, then end up having to mark a majority of those as `not_canonical` later on if a spend of a previously-visited transaction is determined to be a descendant of a `not_canonical` transaction. +* Does not handle transitively-anchored transactions properly. If a transaction is anchored in the best chain, all of it's ancestors are anchored in the best chain even though they do not have an explicit anchor attached. To find transitive anchors, we need to traverse backwards. However this algorithm only traverses forwards. + +#### Option 3: Traverse `TxGraph` backwards, starting from transactions with the highest `last-seen` values. + +The premise is that transactions with higher last-seen values are most likely to be canonical and not conflict with transactions anchored in the best chain (since they are seen most recently in the mempool). + +The algorithm maintains 2 `txid` sets. One for `canonical` and `not_canonical`. These are mutually exclusive sets. A transaction that is included in either of these sets have already been visited and can be skipped. We iterate through all transactions, ordered by descending last-seen values. + +For each transaction, we traverse it's ancestors, stopping when we hit a confirmed transaction or a transaction that conflicts with a confirmed transaction. If a conflict with a confirmed transaction is found, we can mark that transaction and all it's descendants as `not_canonical`. Otherwise, the entire subset will be `canonical`. If we hit a transaction that is anchored in the best chain, we can mark it and all of it's ancestors as `canonical`. + +**Pros:** +* We can efficiently mark large subsets as canonical/not-canonical. + +**Cons:** +* Like option 2, this does not handle transitively-anchored transactions properly. + +#### Option 4: Traverse transactions with anchors first. + +The algorithm's premise is as follows: + +1. If transaction `A` is determined to be canonical, all of `A`'s ancestors must also be canonical. +2. If transaction `B` is determined to be NOT canonical, all of `B`'s descendants must also be NOT canonical. +3. If a transaction is anchored in the best chain, it is canonical. +4. If a transaction conflicts with a canonical transaction, it is NOT canonical. +5. A transaction with a higher last-seen has precedence. +6. Last-seen values are transitive. A transaction's real last-seen value is the max between it's last-seen value all of it's descendants. + +Like Option 3's algorithm, we maintain two mutually-exclusive `txid` sets: `canoncial` and `not_canonical`. + +Imagine a method `mark_canonical(A)` that is based on premise 1 and 2. This method will mark transaction `A` and all of it's ancestors as canonical. For each transaction that is marked canonical, we can iterate all of it's conflicts and mark those as `non_canonical`. If a transaction already exists in `canoncial` or `not_canonical`, we can break early, avoiding duplicate work. + +This algorithm iterates transactions in 3 runs. + +1. Iterate over all transactions with anchors in descending anchor-height order. For any transaction that has an anchor pointing to the best chain, we call `mark_canonical` on it. We iterate in descending-height order to reduce the number of anchors we need to check against the `ChainOracle` (premise 1). The purpose of this run is to populate `non_canonical` with all transactions that directly conflict with anchored transactions and populate `canonical` with all anchored transactions and ancestors of anchors transactions (transitive anchors). +2. Iterate over all transactions with last-seen values, in descending last-seen order. We can call `mark_canonical` on all of these that do not already exist in `canonical` or `not_canonical`. +3. Iterate over remaining transactions that contains anchors (but not in the best chain) and have no last-seen value. We treat these transactions in the same way as we do in run 2. + +**Pros:** +* Transitive anchors are handled correctly. +* We can efficiently mark large subsets as canonical/non-canonical. + +**Cons:** ? + +## Decision Outcome + +Option 4 is implemented in PR #1670. From e34024cd566cd4dc17ab90cc5b191da8e55802e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 3 Dec 2024 16:40:24 +1100 Subject: [PATCH 07/16] feat(chain): Derive `Clone` on `IndexedTxGraph` --- crates/chain/src/indexed_tx_graph.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/chain/src/indexed_tx_graph.rs b/crates/chain/src/indexed_tx_graph.rs index 673cb203e..039924c92 100644 --- a/crates/chain/src/indexed_tx_graph.rs +++ b/crates/chain/src/indexed_tx_graph.rs @@ -13,7 +13,7 @@ use crate::{ /// The [`IndexedTxGraph`] combines a [`TxGraph`] and an [`Indexer`] implementation. /// /// It ensures that [`TxGraph`] and [`Indexer`] are updated atomically. -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct IndexedTxGraph { /// Transaction index. pub index: I, From d4102b4b6bb885934a3bfd8efffeb550541efe7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 3 Dec 2024 15:21:13 +1100 Subject: [PATCH 08/16] perf(chain): add benchmarks for canonicalization logic This is mostly taken from #1735 except we inline many of the functions and test `list_canonical_txs`, `filter_chain_unspents` and `filter_chain_txouts` on all scenarios. CI and README is updated to pin `csv`. Co-authored-by: valued mammal --- .github/workflows/cont_integration.yml | 2 + README.md | 2 + crates/chain/Cargo.toml | 6 +- crates/chain/benches/canonicalization.rs | 250 +++++++++++++++++++++++ crates/wallet/tests/wallet.rs | 2 +- 5 files changed, 260 insertions(+), 2 deletions(-) create mode 100644 crates/chain/benches/canonicalization.rs diff --git a/.github/workflows/cont_integration.yml b/.github/workflows/cont_integration.yml index 2116392d1..e9d8a134b 100644 --- a/.github/workflows/cont_integration.yml +++ b/.github/workflows/cont_integration.yml @@ -51,6 +51,8 @@ jobs: cargo update -p tokio-util --precise "0.7.11" cargo update -p indexmap --precise "2.5.0" cargo update -p security-framework-sys --precise "2.11.1" + cargo update -p csv --precise "1.3.0" + cargo update -p unicode-width --precise "0.1.13" - name: Build run: cargo build --workspace --exclude 'example_*' ${{ matrix.features }} - name: Test diff --git a/README.md b/README.md index dafe923d6..7c8e8a696 100644 --- a/README.md +++ b/README.md @@ -78,6 +78,8 @@ cargo update -p tokio --precise "1.38.1" cargo update -p tokio-util --precise "0.7.11" cargo update -p indexmap --precise "2.5.0" cargo update -p security-framework-sys --precise "2.11.1" +cargo update -p csv --precise "1.3.0" +cargo update -p unicode-width --precise "0.1.13" ``` ## License diff --git a/crates/chain/Cargo.toml b/crates/chain/Cargo.toml index 2dc7d9cc8..4f1a24fa5 100644 --- a/crates/chain/Cargo.toml +++ b/crates/chain/Cargo.toml @@ -28,7 +28,7 @@ rusqlite = { version = "0.31.0", features = ["bundled"], optional = true } rand = "0.8" proptest = "1.2.0" bdk_testenv = { path = "../testenv", default-features = false } - +criterion = { version = "0.2" } [features] default = ["std", "miniscript"] @@ -36,3 +36,7 @@ std = ["bitcoin/std", "miniscript?/std", "bdk_core/std"] serde = ["dep:serde", "bitcoin/serde", "miniscript?/serde", "bdk_core/serde"] hashbrown = ["bdk_core/hashbrown"] rusqlite = ["std", "dep:rusqlite", "serde"] + +[[bench]] +name = "canonicalization" +harness = false diff --git a/crates/chain/benches/canonicalization.rs b/crates/chain/benches/canonicalization.rs new file mode 100644 index 000000000..3002a7ca3 --- /dev/null +++ b/crates/chain/benches/canonicalization.rs @@ -0,0 +1,250 @@ +use bdk_chain::{keychain_txout::KeychainTxOutIndex, local_chain::LocalChain, IndexedTxGraph}; +use bdk_core::{BlockId, CheckPoint}; +use bdk_core::{ConfirmationBlockTime, TxUpdate}; +use bdk_testenv::hash; +use bitcoin::{ + absolute, constants, hashes::Hash, key::Secp256k1, transaction, Amount, BlockHash, Network, + OutPoint, ScriptBuf, Transaction, TxIn, TxOut, +}; +use criterion::{black_box, criterion_group, criterion_main, Criterion}; +use miniscript::{Descriptor, DescriptorPublicKey}; +use std::sync::Arc; + +type Keychain = (); +type KeychainTxGraph = IndexedTxGraph>; + +/// New tx guaranteed to have at least one output +fn new_tx(lt: u32) -> Transaction { + Transaction { + version: transaction::Version::TWO, + lock_time: absolute::LockTime::from_consensus(lt), + input: vec![], + output: vec![TxOut::NULL], + } +} + +fn spk_at_index(txout_index: &KeychainTxOutIndex, index: u32) -> ScriptBuf { + txout_index + .get_descriptor(()) + .unwrap() + .at_derivation_index(index) + .unwrap() + .script_pubkey() +} + +fn genesis_block_id() -> BlockId { + BlockId { + height: 0, + hash: constants::genesis_block(Network::Regtest).block_hash(), + } +} + +fn tip_block_id() -> BlockId { + BlockId { + height: 100, + hash: BlockHash::all_zeros(), + } +} + +/// Add ancestor tx confirmed at `block_id` with `locktime` (used for uniqueness). +/// The transaction always pays 1 BTC to SPK 0. +fn add_ancestor_tx(graph: &mut KeychainTxGraph, block_id: BlockId, locktime: u32) -> OutPoint { + let spk_0 = spk_at_index(&graph.index, 0); + let tx = Transaction { + input: vec![TxIn { + previous_output: OutPoint::new(hash!("bogus"), locktime), + ..Default::default() + }], + output: vec![TxOut { + value: Amount::ONE_BTC, + script_pubkey: spk_0, + }], + ..new_tx(locktime) + }; + let txid = tx.compute_txid(); + let _ = graph.insert_tx(tx); + let _ = graph.insert_anchor( + txid, + ConfirmationBlockTime { + block_id, + confirmation_time: 100, + }, + ); + OutPoint { txid, vout: 0 } +} + +fn setup(f: F) -> (KeychainTxGraph, LocalChain) { + const DESC: &str = "tr([ab28dc00/86h/1h/0h]tpubDCdDtzAMZZrkwKBxwNcGCqe4FRydeD9rfMisoi7qLdraG79YohRfPW4YgdKQhpgASdvh612xXNY5xYzoqnyCgPbkpK4LSVcH5Xv4cK7johH/0/*)"; + let cp = CheckPoint::from_block_ids([genesis_block_id(), tip_block_id()]) + .expect("blocks must be chronological"); + let chain = LocalChain::from_tip(cp).unwrap(); + + let (desc, _) = + >::parse_descriptor(&Secp256k1::new(), DESC).unwrap(); + let mut index = KeychainTxOutIndex::new(10); + index.insert_descriptor((), desc).unwrap(); + let mut tx_graph = KeychainTxGraph::new(index); + + f(&mut tx_graph, &chain); + (tx_graph, chain) +} + +fn run_list_canonical_txs(tx_graph: &KeychainTxGraph, chain: &LocalChain, exp_txs: usize) { + let txs = tx_graph + .graph() + .list_canonical_txs(chain, chain.tip().block_id()); + assert_eq!(txs.count(), exp_txs); +} + +fn run_filter_chain_txouts(tx_graph: &KeychainTxGraph, chain: &LocalChain, exp_txos: usize) { + let utxos = tx_graph.graph().filter_chain_txouts( + chain, + chain.tip().block_id(), + tx_graph.index.outpoints().clone(), + ); + assert_eq!(utxos.count(), exp_txos); +} + +fn run_filter_chain_unspents(tx_graph: &KeychainTxGraph, chain: &LocalChain, exp_utxos: usize) { + let utxos = tx_graph.graph().filter_chain_unspents( + chain, + chain.tip().block_id(), + tx_graph.index.outpoints().clone(), + ); + assert_eq!(utxos.count(), exp_utxos); +} + +pub fn many_conflicting_unconfirmed(c: &mut Criterion) { + const CONFLICTING_TX_COUNT: u32 = 2100; + let (tx_graph, chain) = black_box(setup(|tx_graph, _chain| { + let previous_output = add_ancestor_tx(tx_graph, tip_block_id(), 0); + // Create conflicting txs that spend from `previous_output`. + let spk_1 = spk_at_index(&tx_graph.index, 1); + for i in 1..=CONFLICTING_TX_COUNT { + let tx = Transaction { + input: vec![TxIn { + previous_output, + ..Default::default() + }], + output: vec![TxOut { + value: Amount::ONE_BTC - Amount::from_sat(i as u64 * 10), + script_pubkey: spk_1.clone(), + }], + ..new_tx(i) + }; + let update = TxUpdate { + txs: vec![Arc::new(tx)], + ..Default::default() + }; + let _ = tx_graph.apply_update_at(update, Some(i as u64)); + } + })); + c.bench_function("many_conflicting_unconfirmed::list_canonical_txs", { + let (tx_graph, chain) = (tx_graph.clone(), chain.clone()); + move |b| b.iter(|| run_list_canonical_txs(&tx_graph, &chain, 2)) + }); + c.bench_function("many_conflicting_unconfirmed::filter_chain_txouts", { + let (tx_graph, chain) = (tx_graph.clone(), chain.clone()); + move |b| b.iter(|| run_filter_chain_txouts(&tx_graph, &chain, 2)) + }); + c.bench_function("many_conflicting_unconfirmed::filter_chain_unspents", { + let (tx_graph, chain) = (tx_graph.clone(), chain.clone()); + move |b| b.iter(|| run_filter_chain_unspents(&tx_graph, &chain, 1)) + }); +} + +pub fn many_chained_unconfirmed(c: &mut Criterion) { + const TX_CHAIN_COUNT: u32 = 2100; + let (tx_graph, chain) = black_box(setup(|tx_graph, _chain| { + let mut previous_output = add_ancestor_tx(tx_graph, tip_block_id(), 0); + // Create a chain of unconfirmed txs where each subsequent tx spends the output of the + // previous one. + for i in 0..TX_CHAIN_COUNT { + // Create tx. + let tx = Transaction { + input: vec![TxIn { + previous_output, + ..Default::default() + }], + ..new_tx(i) + }; + let txid = tx.compute_txid(); + let update = TxUpdate { + txs: vec![Arc::new(tx)], + ..Default::default() + }; + let _ = tx_graph.apply_update_at(update, Some(i as u64)); + // Store the next prevout. + previous_output = OutPoint::new(txid, 0); + } + })); + c.bench_function("many_chained_unconfirmed::list_canonical_txs", { + let (tx_graph, chain) = (tx_graph.clone(), chain.clone()); + move |b| b.iter(|| run_list_canonical_txs(&tx_graph, &chain, 2101)) + }); + c.bench_function("many_chained_unconfirmed::filter_chain_txouts", { + let (tx_graph, chain) = (tx_graph.clone(), chain.clone()); + move |b| b.iter(|| run_filter_chain_txouts(&tx_graph, &chain, 1)) + }); + c.bench_function("many_chained_unconfirmed::filter_chain_unspents", { + let (tx_graph, chain) = (tx_graph.clone(), chain.clone()); + move |b| b.iter(|| run_filter_chain_unspents(&tx_graph, &chain, 0)) + }); +} + +pub fn nested_conflicts(c: &mut Criterion) { + const CONFLICTS_PER_OUTPUT: usize = 3; + const GRAPH_DEPTH: usize = 7; + let (tx_graph, chain) = black_box(setup(|tx_graph, _chain| { + let mut prev_ops = core::iter::once(add_ancestor_tx(tx_graph, tip_block_id(), 0)) + .collect::>(); + for depth in 1..GRAPH_DEPTH { + for previous_output in core::mem::take(&mut prev_ops) { + for conflict_i in 1..=CONFLICTS_PER_OUTPUT { + let mut last_seen = depth * conflict_i; + if last_seen % 2 == 0 { + last_seen /= 2; + } + let ((_, script_pubkey), _) = tx_graph.index.next_unused_spk(()).unwrap(); + let value = + Amount::ONE_BTC - Amount::from_sat(depth as u64 * 200 - conflict_i as u64); + let tx = Transaction { + input: vec![TxIn { + previous_output, + ..Default::default() + }], + output: vec![TxOut { + value, + script_pubkey, + }], + ..new_tx(conflict_i as _) + }; + let txid = tx.compute_txid(); + prev_ops.push(OutPoint::new(txid, 0)); + let _ = tx_graph.insert_seen_at(txid, last_seen as _); + let _ = tx_graph.insert_tx(tx); + } + } + } + })); + c.bench_function("nested_conflicts_unconfirmed::list_canonical_txs", { + let (tx_graph, chain) = (tx_graph.clone(), chain.clone()); + move |b| b.iter(|| run_list_canonical_txs(&tx_graph, &chain, GRAPH_DEPTH)) + }); + c.bench_function("nested_conflicts_unconfirmed::filter_chain_txouts", { + let (tx_graph, chain) = (tx_graph.clone(), chain.clone()); + move |b| b.iter(|| run_filter_chain_txouts(&tx_graph, &chain, GRAPH_DEPTH)) + }); + c.bench_function("nested_conflicts_unconfirmed::filter_chain_unspents", { + let (tx_graph, chain) = (tx_graph.clone(), chain.clone()); + move |b| b.iter(|| run_filter_chain_unspents(&tx_graph, &chain, 1)) + }); +} + +criterion_group!( + benches, + many_conflicting_unconfirmed, + many_chained_unconfirmed, + nested_conflicts, +); +criterion_main!(benches); diff --git a/crates/wallet/tests/wallet.rs b/crates/wallet/tests/wallet.rs index 4afb66e65..224929819 100644 --- a/crates/wallet/tests/wallet.rs +++ b/crates/wallet/tests/wallet.rs @@ -267,7 +267,7 @@ fn wallet_should_persist_anchors_and_recover() { .expect("must have loaded changeset"); // stored anchor should be retrieved in the same condition it was persisted if let ChainPosition::Confirmed { - anchor: &obtained_anchor, + anchor: obtained_anchor, .. } = wallet .get_tx(txid) From da0c43ed94b6cc0ecc8ac4f04880fd01a28d244c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Fri, 6 Dec 2024 14:27:24 +1100 Subject: [PATCH 09/16] refactor(chain)!: Rename `LastSeenIn` to `ObservedIn` Also removed extra derives on `ObservedIn` and updated docs for `CanonicalTx`. --- crates/chain/src/canonical_iter.rs | 50 ++++++++++++++++-------------- crates/chain/src/tx_graph.rs | 12 +++---- 2 files changed, 32 insertions(+), 30 deletions(-) diff --git a/crates/chain/src/canonical_iter.rs b/crates/chain/src/canonical_iter.rs index 7077e4721..1157f8d06 100644 --- a/crates/chain/src/canonical_iter.rs +++ b/crates/chain/src/canonical_iter.rs @@ -155,16 +155,16 @@ impl<'g, A: Anchor, C: ChainOracle> Iterator for CanonicalIter<'g, A, C> { if let Some((txid, tx, last_seen)) = self.pending_last_seen.next() { if !self.is_canonicalized(txid) { - let lsi = LastSeenIn::Mempool(last_seen); - self.mark_canonical(tx, CanonicalReason::from_last_seen(lsi)); + let observed_in = ObservedIn::Mempool(last_seen); + self.mark_canonical(tx, CanonicalReason::from_observed_in(observed_in)); } continue; } if let Some((txid, tx, height)) = self.pending_remaining.pop_front() { if !self.is_canonicalized(txid) { - let lsi = LastSeenIn::Block(height); - self.mark_canonical(tx, CanonicalReason::from_last_seen(lsi)); + let observed_in = ObservedIn::Block(height); + self.mark_canonical(tx, CanonicalReason::from_observed_in(observed_in)); } continue; } @@ -174,13 +174,12 @@ impl<'g, A: Anchor, C: ChainOracle> Iterator for CanonicalIter<'g, A, C> { } } -/// Represents when and where a given transaction is last seen. -#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, core::hash::Hash)] -#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] -pub enum LastSeenIn { - /// The transaction was last seen in a block of height. +/// Represents when and where a transaction was last observed in. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +pub enum ObservedIn { + /// The transaction was last observed in a block of height. Block(u32), - /// The transaction was last seen in the mempool at the given unix timestamp. + /// The transaction was last observed in the mempool at the given unix timestamp. Mempool(u64), } @@ -194,12 +193,12 @@ pub enum CanonicalReason { /// Whether the anchor is of the transaction's descendant. descendant: Option, }, - /// This transaction does not conflict with any other transaction with a more recent `last_seen` - /// value or one that is anchored in the best chain. - LastSeen { - /// The [`LastSeenIn`] value of the transaction. - last_seen: LastSeenIn, - /// Whether the [`LastSeenIn`] value is of the transaction's descendant. + /// This transaction does not conflict with any other transaction with a more recent + /// [`ObservedIn`] value or one that is anchored in the best chain. + ObservedIn { + /// The [`ObservedIn`] value of the transaction. + observed_in: ObservedIn, + /// Whether the [`ObservedIn`] value is of the transaction's descendant. descendant: Option, }, } @@ -214,16 +213,16 @@ impl CanonicalReason { } /// Constructs a [`CanonicalReason`] from a `last_seen` value. - pub fn from_last_seen(last_seen: LastSeenIn) -> Self { - Self::LastSeen { - last_seen, + pub fn from_observed_in(observed_in: ObservedIn) -> Self { + Self::ObservedIn { + observed_in, descendant: None, } } /// Contruct a new [`CanonicalReason`] from the original which is transitive to `descendant`. /// - /// This signals that either the [`LastSeenIn`] or [`Anchor`] value belongs to the transaction's + /// This signals that either the [`ObservedIn`] or [`Anchor`] value belongs to the transaction's /// descendant, but is transitively relevant. pub fn to_transitive(&self, descendant: Txid) -> Self { match self { @@ -231,19 +230,22 @@ impl CanonicalReason { anchor: anchor.clone(), descendant: Some(descendant), }, - CanonicalReason::LastSeen { last_seen, .. } => Self::LastSeen { - last_seen: *last_seen, + CanonicalReason::ObservedIn { + observed_in: last_seen, + .. + } => Self::ObservedIn { + observed_in: *last_seen, descendant: Some(descendant), }, } } - /// This signals that either the [`LastSeenIn`] or [`Anchor`] value belongs to the transaction's + /// This signals that either the [`ObservedIn`] or [`Anchor`] value belongs to the transaction's /// descendant. pub fn descendant(&self) -> &Option { match self { CanonicalReason::Anchor { descendant, .. } => descendant, - CanonicalReason::LastSeen { descendant, .. } => descendant, + CanonicalReason::ObservedIn { descendant, .. } => descendant, } } } diff --git a/crates/chain/src/tx_graph.rs b/crates/chain/src/tx_graph.rs index 71fde7188..df3ff4e4a 100644 --- a/crates/chain/src/tx_graph.rs +++ b/crates/chain/src/tx_graph.rs @@ -94,7 +94,7 @@ use crate::collections::*; use crate::BlockId; use crate::CanonicalIter; use crate::CanonicalReason; -use crate::LastSeenIn; +use crate::ObservedIn; use crate::{Anchor, Balance, ChainOracle, ChainPosition, FullTxOut, Merge}; use alloc::collections::vec_deque::VecDeque; use alloc::sync::Arc; @@ -207,10 +207,10 @@ impl Default for TxNodeInternal { } } -/// A transaction that is included in the chain, or is still in mempool. +/// A transaction that is deemed to be part of the canonical history. #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] pub struct CanonicalTx<'a, T, A> { - /// How the transaction is observed as (confirmed or unconfirmed). + /// How the transaction is observed in the canonical chain (confirmed or unconfirmed). pub chain_position: ChainPosition, /// The transaction node (as part of the graph). pub tx_node: TxNode<'a, T, A>, @@ -840,11 +840,11 @@ impl TxGraph { transitively: None, }, }, - CanonicalReason::LastSeen { last_seen, .. } => match last_seen { - LastSeenIn::Mempool(last_seen) => ChainPosition::Unconfirmed { + CanonicalReason::ObservedIn { observed_in, .. } => match observed_in { + ObservedIn::Mempool(last_seen) => ChainPosition::Unconfirmed { last_seen: Some(last_seen), }, - LastSeenIn::Block(_) => ChainPosition::Unconfirmed { last_seen: None }, + ObservedIn::Block(_) => ChainPosition::Unconfirmed { last_seen: None }, }, }; Ok(CanonicalTx { From 68f7b7724388eb104775a256b3529198e5af84b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Fri, 6 Dec 2024 16:35:56 +1100 Subject: [PATCH 10/16] test(chain): Add canonicalization test Tx anchored in orphaned block and not seen in the mempool should be canon. --- crates/chain/tests/test_tx_graph_conflicts.rs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/crates/chain/tests/test_tx_graph_conflicts.rs b/crates/chain/tests/test_tx_graph_conflicts.rs index a2b6abdc3..ff4c8b1f9 100644 --- a/crates/chain/tests/test_tx_graph_conflicts.rs +++ b/crates/chain/tests/test_tx_graph_conflicts.rs @@ -663,6 +663,29 @@ fn test_tx_conflict_handling() { confirmed: Amount::from_sat(8000), ..Default::default() } + }, + Scenario { + name: "tx anchored in orphaned block and not seen in mempool should be canon", + tx_templates: &[ + TxTemplate { + tx_name: "root", + inputs: &[TxInTemplate::Bogus], + outputs: &[TxOutTemplate::new(10_000, None)], + anchors: &[block_id!(1, "B")], + ..Default::default() + }, + TxTemplate { + tx_name: "tx", + inputs: &[TxInTemplate::PrevTx("root", 0)], + outputs: &[TxOutTemplate::new(9000, Some(0))], + anchors: &[block_id!(6, "not G")], + ..Default::default() + }, + ], + exp_chain_txs: HashSet::from(["root", "tx"]), + exp_chain_txouts: HashSet::from([("tx", 0)]), + exp_unspents: HashSet::from([("tx", 0)]), + exp_balance: Balance { trusted_pending: Amount::from_sat(9000), ..Default::default() } } ]; From 4706315e8f60a7d308184aa028f51741b47219a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 10 Dec 2024 20:49:47 +1100 Subject: [PATCH 11/16] chore(chain): Address `CanonicalIter` nitpicks --- crates/chain/src/canonical_iter.rs | 48 +++++++++++++++--------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/crates/chain/src/canonical_iter.rs b/crates/chain/src/canonical_iter.rs index 1157f8d06..6a12bbf25 100644 --- a/crates/chain/src/canonical_iter.rs +++ b/crates/chain/src/canonical_iter.rs @@ -13,9 +13,10 @@ pub struct CanonicalIter<'g, A, C> { chain: &'g C, chain_tip: BlockId, - pending_anchored: Box, &'g BTreeSet)> + 'g>, - pending_last_seen: Box, u64)> + 'g>, - pending_remaining: VecDeque<(Txid, Arc, u32)>, + unprocessed_txs_with_anchors: + Box, &'g BTreeSet)> + 'g>, + unprocessed_txs_with_last_seens: Box, u64)> + 'g>, + unprocessed_txs_left_over: VecDeque<(Txid, Arc, u32)>, canonical: HashMap, CanonicalReason)>, not_canonical: HashSet, @@ -41,9 +42,9 @@ impl<'g, A: Anchor, C: ChainOracle> CanonicalIter<'g, A, C> { tx_graph, chain, chain_tip, - pending_anchored, - pending_last_seen, - pending_remaining: VecDeque::new(), + unprocessed_txs_with_anchors: pending_anchored, + unprocessed_txs_with_last_seens: pending_last_seen, + unprocessed_txs_left_over: VecDeque::new(), canonical: HashMap::new(), not_canonical: HashSet::new(), queue: VecDeque::new(), @@ -67,18 +68,20 @@ impl<'g, A: Anchor, C: ChainOracle> CanonicalIter<'g, A, C> { .chain .is_block_in_chain(anchor.anchor_block(), self.chain_tip)?; if in_chain_opt == Some(true) { - self.mark_canonical(tx, CanonicalReason::from_anchor(anchor.clone())); + self.mark_canonical(txid, tx, CanonicalReason::from_anchor(anchor.clone())); return Ok(()); } } // cannot determine - self.pending_remaining.push_back(( + self.unprocessed_txs_left_over.push_back(( txid, tx, anchors .iter() .last() - .unwrap() + .expect( + "tx taken from `unprocessed_txs_with_anchors` so it must atleast have an anchor", + ) .confirmation_height_upper_bound(), )); Ok(()) @@ -86,8 +89,8 @@ impl<'g, A: Anchor, C: ChainOracle> CanonicalIter<'g, A, C> { /// Marks a transaction and it's ancestors as canoncial. Mark all conflicts of these as /// `not_canonical`. - fn mark_canonical(&mut self, tx: Arc, reason: CanonicalReason) { - let starting_txid = tx.compute_txid(); + fn mark_canonical(&mut self, txid: Txid, tx: Arc, reason: CanonicalReason) { + let starting_txid = txid; let mut is_root = true; TxAncestors::new_include_root( self.tx_graph, @@ -126,11 +129,11 @@ impl<'g, A: Anchor, C: ChainOracle> CanonicalIter<'g, A, C> { Some(()) }, ) - .for_each(|_| {}) + .run_until_finished() } } -impl<'g, A: Anchor, C: ChainOracle> Iterator for CanonicalIter<'g, A, C> { +impl Iterator for CanonicalIter<'_, A, C> { type Item = Result<(Txid, Arc, CanonicalReason), C::Error>; fn next(&mut self) -> Option { @@ -144,7 +147,7 @@ impl<'g, A: Anchor, C: ChainOracle> Iterator for CanonicalIter<'g, A, C> { return Some(Ok((txid, tx, reason))); } - if let Some((txid, tx, anchors)) = self.pending_anchored.next() { + if let Some((txid, tx, anchors)) = self.unprocessed_txs_with_anchors.next() { if !self.is_canonicalized(txid) { if let Err(err) = self.scan_anchors(txid, tx, anchors) { return Some(Err(err)); @@ -153,18 +156,18 @@ impl<'g, A: Anchor, C: ChainOracle> Iterator for CanonicalIter<'g, A, C> { continue; } - if let Some((txid, tx, last_seen)) = self.pending_last_seen.next() { + if let Some((txid, tx, last_seen)) = self.unprocessed_txs_with_last_seens.next() { if !self.is_canonicalized(txid) { let observed_in = ObservedIn::Mempool(last_seen); - self.mark_canonical(tx, CanonicalReason::from_observed_in(observed_in)); + self.mark_canonical(txid, tx, CanonicalReason::from_observed_in(observed_in)); } continue; } - if let Some((txid, tx, height)) = self.pending_remaining.pop_front() { + if let Some((txid, tx, height)) = self.unprocessed_txs_left_over.pop_front() { if !self.is_canonicalized(txid) { let observed_in = ObservedIn::Block(height); - self.mark_canonical(tx, CanonicalReason::from_observed_in(observed_in)); + self.mark_canonical(txid, tx, CanonicalReason::from_observed_in(observed_in)); } continue; } @@ -212,7 +215,7 @@ impl CanonicalReason { } } - /// Constructs a [`CanonicalReason`] from a `last_seen` value. + /// Constructs a [`CanonicalReason`] from an `observed_in` value. pub fn from_observed_in(observed_in: ObservedIn) -> Self { Self::ObservedIn { observed_in, @@ -230,11 +233,8 @@ impl CanonicalReason { anchor: anchor.clone(), descendant: Some(descendant), }, - CanonicalReason::ObservedIn { - observed_in: last_seen, - .. - } => Self::ObservedIn { - observed_in: *last_seen, + CanonicalReason::ObservedIn { observed_in, .. } => Self::ObservedIn { + observed_in: *observed_in, descendant: Some(descendant), }, } From 1196405904acdaa2287e4edc0cc71a71cc9790cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 10 Dec 2024 21:34:07 +1100 Subject: [PATCH 12/16] refactor(chain): Reorganize `TxGraph::insert_anchor` logic for clarity --- crates/chain/src/tx_graph.rs | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/crates/chain/src/tx_graph.rs b/crates/chain/src/tx_graph.rs index df3ff4e4a..6659e0352 100644 --- a/crates/chain/src/tx_graph.rs +++ b/crates/chain/src/tx_graph.rs @@ -634,20 +634,25 @@ impl TxGraph { /// The [`ChangeSet`] returned will be empty if graph already knows that `txid` exists in /// `anchor`. pub fn insert_anchor(&mut self, txid: Txid, anchor: A) -> ChangeSet { + // These two variables are used to determine how to modify the `txid`'s entry in + // `txs_by_highest_conf_heights`. + // We want to remove `(old_top_h?, txid)` and insert `(new_top_h?, txid)`. let mut old_top_h = None; let mut new_top_h = anchor.confirmation_height_upper_bound(); let is_changed = match self.anchors.entry(txid) { hash_map::Entry::Occupied(mut e) => { - fn top_anchor_height(anchors: &BTreeSet) -> Option { - anchors - .iter() - .last() - .map(Anchor::confirmation_height_upper_bound) + old_top_h = e + .get() + .iter() + .last() + .map(Anchor::confirmation_height_upper_bound); + if let Some(old_top_h) = old_top_h { + if old_top_h > new_top_h { + new_top_h = old_top_h; + } } - old_top_h = top_anchor_height(e.get()); let is_changed = e.get_mut().insert(anchor.clone()); - new_top_h = top_anchor_height(e.get()).expect("must have anchor"); is_changed } hash_map::Entry::Vacant(e) => { @@ -658,7 +663,12 @@ impl TxGraph { let mut changeset = ChangeSet::::default(); if is_changed { - if old_top_h != Some(new_top_h) { + let new_top_is_changed = match old_top_h { + None => true, + Some(old_top_h) if old_top_h != new_top_h => true, + _ => false, + }; + if new_top_is_changed { if let Some(prev_top_h) = old_top_h { self.txs_by_highest_conf_heights.remove(&(prev_top_h, txid)); } From caa0f13d40d000229dd867e6c52039a0c897fed2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 10 Dec 2024 21:56:37 +1100 Subject: [PATCH 13/16] docs(wallet): Explain `.take` usage --- crates/wallet/src/wallet/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs index 542db3b06..98559d4d9 100644 --- a/crates/wallet/src/wallet/mod.rs +++ b/crates/wallet/src/wallet/mod.rs @@ -1844,6 +1844,9 @@ impl Wallet { .graph() .list_canonical_txs(&self.chain, chain_tip) .filter(|canon_tx| prev_txids.contains(&canon_tx.tx_node.txid)) + // This is for a small performance gain. Although `.filter` filters out excess txs, it + // will still consume the internal `CanonicalIter` entirely. Having a `.take` here + // allows us to stop further unnecessary canonicalization. .take(prev_txids.len()) .map(|canon_tx| { let txid = canon_tx.tx_node.txid; From af92199b69642d60aec509b246b2252c6a41d220 Mon Sep 17 00:00:00 2001 From: Jiri Jakes Date: Thu, 5 Dec 2024 16:51:37 +0800 Subject: [PATCH 14/16] refactor(wallet): Reuse chain position instead of obtaining new one In `Wallet::preselect_utxos()`, the code used to obtain chain position of the UTXO's transaction from the graph, however the chain position is already recorded within the UTXO's representation (`LocalOutput`). This patch reuses the existing chain position instead of obtaining a fresh one. --- crates/wallet/src/wallet/mod.rs | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/crates/wallet/src/wallet/mod.rs b/crates/wallet/src/wallet/mod.rs index 98559d4d9..05bac3d93 100644 --- a/crates/wallet/src/wallet/mod.rs +++ b/crates/wallet/src/wallet/mod.rs @@ -2011,7 +2011,6 @@ impl Wallet { let must_only_use_confirmed_tx = bumping_fee.is_some(); let must_use_all_available = *drain_wallet; - let chain_tip = self.chain.tip().block_id(); // must_spend <- manually selected utxos // may_spend <- all other available utxos let mut may_spend = self.get_available_utxos(); @@ -2029,27 +2028,18 @@ impl Wallet { return (must_spend, vec![]); } - let canon_txs = self - .indexed_graph - .graph() - .list_canonical_txs(&self.chain, chain_tip) - .map(|canon_tx| (canon_tx.tx_node.txid, canon_tx)) - .collect::>(); - let satisfies_confirmed = may_spend .iter() .map(|u| -> bool { let txid = u.0.outpoint.txid; - let (chain_position, tx) = match canon_txs.get(&txid) { - Some(CanonicalTx { - chain_position, - tx_node, - }) => (chain_position, tx_node.tx.clone()), + let tx = match self.indexed_graph.graph().get_tx(txid) { + Some(tx) => tx, None => return false, }; // Whether the UTXO is mature and, if needed, confirmed let mut spendable = true; + let chain_position = u.0.chain_position; if must_only_use_confirmed_tx && !chain_position.is_confirmed() { return false; } From 1508ae3db4b0595e61136d4d28e10777c92a7273 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 10 Dec 2024 22:53:12 +1100 Subject: [PATCH 15/16] chore: Fix typos --- crates/chain/src/canonical_iter.rs | 2 +- docs/adr/0003_canonicalization_algorithm.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/chain/src/canonical_iter.rs b/crates/chain/src/canonical_iter.rs index 6a12bbf25..99550ab7f 100644 --- a/crates/chain/src/canonical_iter.rs +++ b/crates/chain/src/canonical_iter.rs @@ -87,7 +87,7 @@ impl<'g, A: Anchor, C: ChainOracle> CanonicalIter<'g, A, C> { Ok(()) } - /// Marks a transaction and it's ancestors as canoncial. Mark all conflicts of these as + /// Marks a transaction and it's ancestors as canonical. Mark all conflicts of these as /// `not_canonical`. fn mark_canonical(&mut self, txid: Txid, tx: Arc, reason: CanonicalReason) { let starting_txid = txid; diff --git a/docs/adr/0003_canonicalization_algorithm.md b/docs/adr/0003_canonicalization_algorithm.md index 503e857e6..c3062cbc1 100644 --- a/docs/adr/0003_canonicalization_algorithm.md +++ b/docs/adr/0003_canonicalization_algorithm.md @@ -62,9 +62,9 @@ The algorithm's premise is as follows: 5. A transaction with a higher last-seen has precedence. 6. Last-seen values are transitive. A transaction's real last-seen value is the max between it's last-seen value all of it's descendants. -Like Option 3's algorithm, we maintain two mutually-exclusive `txid` sets: `canoncial` and `not_canonical`. +Like Option 3's algorithm, we maintain two mutually-exclusive `txid` sets: `canonical` and `not_canonical`. -Imagine a method `mark_canonical(A)` that is based on premise 1 and 2. This method will mark transaction `A` and all of it's ancestors as canonical. For each transaction that is marked canonical, we can iterate all of it's conflicts and mark those as `non_canonical`. If a transaction already exists in `canoncial` or `not_canonical`, we can break early, avoiding duplicate work. +Imagine a method `mark_canonical(A)` that is based on premise 1 and 2. This method will mark transaction `A` and all of it's ancestors as canonical. For each transaction that is marked canonical, we can iterate all of it's conflicts and mark those as `non_canonical`. If a transaction already exists in `canonical` or `not_canonical`, we can break early, avoiding duplicate work. This algorithm iterates transactions in 3 runs. From 956d0a93bab36fd919e81718832fbc1033b677e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 10 Dec 2024 22:57:41 +1100 Subject: [PATCH 16/16] test(chain): Update test docs to stop referencing `get_chain_position` --- crates/chain/tests/test_indexed_tx_graph.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/chain/tests/test_indexed_tx_graph.rs b/crates/chain/tests/test_indexed_tx_graph.rs index 882041fba..1e28eb6a2 100644 --- a/crates/chain/tests/test_indexed_tx_graph.rs +++ b/crates/chain/tests/test_indexed_tx_graph.rs @@ -525,8 +525,8 @@ fn test_list_owned_txouts() { } /// Given a `LocalChain`, `IndexedTxGraph`, and a `Transaction`, when we insert some anchor -/// (possibly non-canonical) and/or a last-seen timestamp into the graph, we expect the -/// result of `get_chain_position` in these cases: +/// (possibly non-canonical) and/or a last-seen timestamp into the graph, we check the canonical +/// position of the tx: /// /// - tx with no anchors or last_seen has no `ChainPosition` /// - tx with any last_seen will be `Unconfirmed` @@ -561,9 +561,8 @@ fn test_get_chain_position() { let cp = CheckPoint::from_block_ids(blocks.clone()).unwrap(); let chain = LocalChain::from_tip(cp).unwrap(); - // The test will insert a transaction into the indexed tx graph - // along with any anchors and timestamps, then check the value - // returned by `get_chain_position`. + // The test will insert a transaction into the indexed tx graph along with any anchors and + // timestamps, then check the tx's canonical position is expected. fn run( chain: &LocalChain, graph: &mut IndexedTxGraph>,