diff --git a/crates/chain/src/canonical_iter.rs b/crates/chain/src/canonical_iter.rs index 99550ab7f..3e3cd6a48 100644 --- a/crates/chain/src/canonical_iter.rs +++ b/crates/chain/src/canonical_iter.rs @@ -1,12 +1,16 @@ -use crate::collections::{hash_map, HashMap, HashSet, VecDeque}; +use crate::collections::{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 alloc::vec::Vec; use bdk_core::BlockId; use bitcoin::{Transaction, Txid}; +type CanonicalMap = HashMap, CanonicalReason)>; +type NotCanonicalSet = HashSet; + /// Iterates over canonical txs. pub struct CanonicalIter<'g, A, C> { tx_graph: &'g TxGraph, @@ -18,8 +22,8 @@ pub struct CanonicalIter<'g, A, C> { unprocessed_txs_with_last_seens: Box, u64)> + 'g>, unprocessed_txs_left_over: VecDeque<(Txid, Arc, u32)>, - canonical: HashMap, CanonicalReason)>, - not_canonical: HashSet, + canonical: CanonicalMap, + not_canonical: NotCanonicalSet, queue: VecDeque, } @@ -87,27 +91,49 @@ impl<'g, A: Anchor, C: ChainOracle> CanonicalIter<'g, A, C> { Ok(()) } - /// Marks a transaction and it's ancestors as canonical. Mark all conflicts of these as + /// Marks `tx` and it's ancestors as canonical and mark all conflicts of these as /// `not_canonical`. + /// + /// The exception is when it is discovered that `tx` double spends itself (i.e. two of it's + /// inputs conflict with each other), then no changes will be made. + /// + /// The logic works by having two loops where one is nested in another. + /// * The outer loop iterates through ancestors of `tx` (including `tx`). We can transitively + /// assume that all ancestors of `tx` are also canonical. + /// * The inner loop loops through conflicts of ancestors of `tx`. Any descendants of conflicts + /// are also conflicts and are transitively considered non-canonical. + /// + /// If the inner loop ends up marking `tx` as non-canonical, then we know that it double spends + /// itself. fn mark_canonical(&mut self, txid: Txid, tx: Arc, reason: CanonicalReason) { let starting_txid = txid; - let mut is_root = true; - TxAncestors::new_include_root( + let mut is_starting_tx = true; + + // We keep track of changes made so far so that we can undo it later in case we detect that + // `tx` double spends itself. + let mut detected_self_double_spend = false; + let mut undo_not_canonical = Vec::::new(); + + // `staged_queue` doubles as the `undo_canonical` data. + let staged_queue = TxAncestors::new_include_root( self.tx_graph, tx, - |_: usize, tx: Arc| -> Option<()> { + |_: usize, tx: Arc| -> Option { let this_txid = tx.compute_txid(); - let this_reason = if is_root { - is_root = false; + let this_reason = if is_starting_tx { + is_starting_tx = false; reason.clone() } else { reason.to_transitive(starting_txid) }; + + use crate::collections::hash_map::Entry; 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, + Entry::Occupied(_) => return None, + 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) { @@ -116,6 +142,7 @@ impl<'g, A: Anchor, C: ChainOracle> CanonicalIter<'g, A, C> { conflict_txid, |_: usize, txid: Txid| -> Option<()> { if self.not_canonical.insert(txid) { + undo_not_canonical.push(txid); Some(()) } else { None @@ -124,12 +151,28 @@ impl<'g, A: Anchor, C: ChainOracle> CanonicalIter<'g, A, C> { ) .run_until_finished() } + + if self.not_canonical.contains(&this_txid) { + // Early exit if self-double-spend is detected. + detected_self_double_spend = true; + return None; + } canonical_entry.insert((tx, this_reason)); - self.queue.push_back(this_txid); - Some(()) + Some(this_txid) }, ) - .run_until_finished() + .collect::>(); + + if detected_self_double_spend { + for txid in staged_queue { + self.canonical.remove(&txid); + } + for txid in undo_not_canonical { + self.not_canonical.remove(&txid); + } + } else { + self.queue.extend(staged_queue); + } } } diff --git a/crates/chain/tests/test_tx_graph_conflicts.rs b/crates/chain/tests/test_tx_graph_conflicts.rs index ff4c8b1f9..4de657359 100644 --- a/crates/chain/tests/test_tx_graph_conflicts.rs +++ b/crates/chain/tests/test_tx_graph_conflicts.rs @@ -686,7 +686,111 @@ fn test_tx_conflict_handling() { exp_chain_txouts: HashSet::from([("tx", 0)]), exp_unspents: HashSet::from([("tx", 0)]), exp_balance: Balance { trusted_pending: Amount::from_sat(9000), ..Default::default() } - } + }, + Scenario { + name: "tx spends from 2 conflicting transactions where a conflict spends another", + tx_templates: &[ + TxTemplate { + tx_name: "A", + inputs: &[TxInTemplate::Bogus], + outputs: &[TxOutTemplate::new(10_000, None)], + last_seen: Some(1), + ..Default::default() + }, + TxTemplate { + tx_name: "S1", + inputs: &[TxInTemplate::PrevTx("A", 0)], + outputs: &[TxOutTemplate::new(9_000, None)], + last_seen: Some(2), + ..Default::default() + }, + TxTemplate { + tx_name: "S2", + inputs: &[TxInTemplate::PrevTx("A", 0), TxInTemplate::PrevTx("S1", 0)], + outputs: &[TxOutTemplate::new(17_000, None)], + last_seen: Some(3), + ..Default::default() + }, + ], + exp_chain_txs: HashSet::from(["A", "S1"]), + exp_chain_txouts: HashSet::from([]), + exp_unspents: HashSet::from([]), + exp_balance: Balance::default(), + }, + Scenario { + name: "tx spends from 2 conflicting transactions where the conflict is nested", + tx_templates: &[ + TxTemplate { + tx_name: "A", + inputs: &[TxInTemplate::Bogus], + outputs: &[TxOutTemplate::new(10_000, Some(0))], + last_seen: Some(1), + ..Default::default() + }, + TxTemplate { + tx_name: "S1", + inputs: &[TxInTemplate::PrevTx("A", 0)], + outputs: &[TxOutTemplate::new(9_000, Some(0))], + last_seen: Some(3), + ..Default::default() + }, + TxTemplate { + tx_name: "B", + inputs: &[TxInTemplate::PrevTx("S1", 0)], + outputs: &[TxOutTemplate::new(8_000, Some(0))], + last_seen: Some(2), + ..Default::default() + }, + TxTemplate { + tx_name: "S2", + inputs: &[TxInTemplate::PrevTx("B", 0), TxInTemplate::PrevTx("A", 0)], + outputs: &[TxOutTemplate::new(17_000, Some(0))], + last_seen: Some(4), + ..Default::default() + }, + ], + exp_chain_txs: HashSet::from(["A", "S1", "B"]), + exp_chain_txouts: HashSet::from([("A", 0), ("B", 0), ("S1", 0)]), + exp_unspents: HashSet::from([("B", 0)]), + exp_balance: Balance { trusted_pending: Amount::from_sat(8_000), ..Default::default() }, + }, + Scenario { + name: "tx spends from 2 conflicting transactions where the conflict is nested (different last_seens)", + tx_templates: &[ + TxTemplate { + tx_name: "A", + inputs: &[TxInTemplate::Bogus], + outputs: &[TxOutTemplate::new(10_000, Some(0))], + last_seen: Some(1), + ..Default::default() + }, + TxTemplate { + tx_name: "S1", + inputs: &[TxInTemplate::PrevTx("A", 0)], + outputs: &[TxOutTemplate::new(9_000, Some(0))], + last_seen: Some(4), + ..Default::default() + }, + TxTemplate { + tx_name: "B", + inputs: &[TxInTemplate::PrevTx("S1", 0)], + outputs: &[TxOutTemplate::new(8_000, Some(0))], + last_seen: Some(2), + ..Default::default() + }, + TxTemplate { + tx_name: "S2", + inputs: &[TxInTemplate::PrevTx("A", 0), TxInTemplate::PrevTx("B", 0)], + outputs: &[TxOutTemplate::new(17_000, Some(0))], + last_seen: Some(3), + ..Default::default() + }, + ], + exp_chain_txs: HashSet::from(["A", "S1", "B"]), + exp_chain_txouts: HashSet::from([("A", 0), ("B", 0), ("S1", 0)]), + exp_unspents: HashSet::from([("B", 0)]), + exp_balance: Balance { trusted_pending: Amount::from_sat(8_000), ..Default::default() }, + }, ]; for scenario in scenarios {