Skip to content

Commit d57a580

Browse files
evanlinjinclaude
authored andcommitted
refactor(chain): Replace canonical_ancestors with canonical_spends
As suggested in PR review, this refactors CanonicalIter to use `canonical_spends: HashMap<OutPoint, Txid>` instead of `canonical_ancestors: HashMap<Txid, Vec<Txid>>`. Benefits: - Simpler data structure that directly tracks which tx spends each output - Clearer and easier to understand code - Performance neutral to slightly positive in most scenarios Benchmark results show: - Conflicting txs: 0.2-1.6% improvement - Chained txs: Mixed (list ops ~2% faster, filter ops 2-4% slower) - Nested conflicts: 0.5-1.1% improvement Overall performance impact is minimal (within ±4%) while code clarity is significantly improved. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 0aab769 commit d57a580

File tree

1 file changed

+29
-27
lines changed

1 file changed

+29
-27
lines changed

crates/chain/src/canonical_iter.rs

Lines changed: 29 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use alloc::collections::BTreeSet;
66
use alloc::sync::Arc;
77
use alloc::vec::Vec;
88
use bdk_core::BlockId;
9-
use bitcoin::{Transaction, Txid};
9+
use bitcoin::{OutPoint, Transaction, Txid};
1010

1111
type CanonicalMap<A> = HashMap<Txid, (Arc<Transaction>, CanonicalReason<A>)>;
1212
type NotCanonicalSet = HashSet<Txid>;
@@ -36,7 +36,7 @@ pub struct CanonicalIter<'g, A, C> {
3636
canonical: CanonicalMap<A>,
3737
not_canonical: NotCanonicalSet,
3838

39-
canonical_ancestors: HashMap<Txid, Vec<Txid>>,
39+
canonical_spends: HashMap<OutPoint, Txid>,
4040
canonical_roots: VecDeque<Txid>,
4141

4242
queue: VecDeque<Txid>,
@@ -78,7 +78,7 @@ impl<'g, A: Anchor, C: ChainOracle> CanonicalIter<'g, A, C> {
7878
unprocessed_leftover_txs: VecDeque::new(),
7979
canonical: HashMap::new(),
8080
not_canonical: HashSet::new(),
81-
canonical_ancestors: HashMap::new(),
81+
canonical_spends: HashMap::new(),
8282
canonical_roots: VecDeque::new(),
8383
queue: VecDeque::new(),
8484
}
@@ -187,16 +187,13 @@ impl<'g, A: Anchor, C: ChainOracle> CanonicalIter<'g, A, C> {
187187
return None;
188188
}
189189

190-
// Calculates all the existing ancestors for the given Txid
191-
self.canonical_ancestors.insert(
192-
this_txid,
193-
tx.clone()
194-
.input
195-
.iter()
196-
.filter(|txin| self.tx_graph.get_tx(txin.previous_output.txid).is_some())
197-
.map(|txin| txin.previous_output.txid)
198-
.collect(),
199-
);
190+
// Record each input's outpoint as being spent by this transaction
191+
for input in &tx.input {
192+
if self.tx_graph.get_tx(input.previous_output.txid).is_some() {
193+
self.canonical_spends
194+
.insert(input.previous_output, this_txid);
195+
}
196+
}
200197

201198
canonical_entry.insert((tx, this_reason));
202199
Some(this_txid)
@@ -206,20 +203,23 @@ impl<'g, A: Anchor, C: ChainOracle> CanonicalIter<'g, A, C> {
206203

207204
if detected_self_double_spend {
208205
for txid in staged_queue {
209-
self.canonical.remove(&txid);
210-
self.canonical_ancestors.remove(&txid);
206+
if let Some((tx, _)) = self.canonical.remove(&txid) {
207+
// Remove all the spends that were added for this transaction
208+
for input in &tx.input {
209+
self.canonical_spends.remove(&input.previous_output);
210+
}
211+
}
211212
}
212213
for txid in undo_not_canonical {
213214
self.not_canonical.remove(&txid);
214215
}
215216
} else {
216217
for txid in staged_queue {
217218
let tx = self.tx_graph.get_tx(txid).expect("tx must exist");
218-
let has_no_ancestors = self
219-
.canonical_ancestors
220-
.get(&txid)
221-
.expect("should exist")
222-
.is_empty();
219+
let has_no_ancestors = tx
220+
.input
221+
.iter()
222+
.all(|input| self.tx_graph.get_tx(input.previous_output.txid).is_none());
223223

224224
// check if it's a root: it's either a coinbase transaction or has no known
225225
// ancestors in the tx_graph.
@@ -269,7 +269,7 @@ impl<A: Anchor, C: ChainOracle> Iterator for CanonicalIter<'_, A, C> {
269269

270270
if !self.canonical_roots.is_empty() {
271271
let topological_iter = TopologicalIter::new(
272-
&self.canonical_ancestors,
272+
&self.canonical_spends,
273273
self.canonical_roots.drain(..).collect(),
274274
|txid| {
275275
let tx_node = self.tx_graph.get_tx_node(txid).expect("tx should exist");
@@ -405,15 +405,17 @@ impl<F> TopologicalIter<F>
405405
where
406406
F: FnMut(bitcoin::Txid) -> u32,
407407
{
408-
fn new(ancestors: &HashMap<Txid, Vec<Txid>>, roots: Vec<Txid>, mut sort_by: F) -> Self {
408+
fn new(canonical_spends: &HashMap<OutPoint, Txid>, roots: Vec<Txid>, mut sort_by: F) -> Self {
409409
let mut inputs_count = HashMap::new();
410410
let mut adj_list: HashMap<Txid, Vec<Txid>> = HashMap::new();
411411

412-
for (txid, ancestors) in ancestors {
413-
for ancestor in ancestors {
414-
adj_list.entry(*ancestor).or_default().push(*txid);
415-
*inputs_count.entry(*txid).or_insert(0) += 1;
416-
}
412+
// Build adjacency list from canonical_spends
413+
// Each entry in canonical_spends tells us that outpoint is spent by txid
414+
// So if outpoint's txid exists, we create an edge from that tx to the spending tx
415+
for (outpoint, spending_txid) in canonical_spends {
416+
let ancestor_txid = outpoint.txid;
417+
adj_list.entry(ancestor_txid).or_default().push(*spending_txid);
418+
*inputs_count.entry(*spending_txid).or_insert(0) += 1;
417419
}
418420

419421
let mut current_level: Vec<Txid> = roots.to_vec();

0 commit comments

Comments
 (0)