Skip to content

Commit 876bba9

Browse files
oleonardolimaclaude
andcommitted
feat(chain): add topological ordering for canonical transactions
It deprecates the existing `list_canonical_txs` and `try_list_canonical_txs` in favor of `list_ordered_canonical_txs` which guarantees topological ordering. The new `list_ordered_canonical_txs` ensures that spending transactions always appear after their inputs, topologically ordered in "spending order". - Add `TopologicalIterator` for level-based topological sorting - Use the `ChainPosition` for sorting within the same level - Add the new method to the canonicalization benchmarks - Update the new test to verify topological ordering correctness Co-Authored-By: Claude <[email protected]>
1 parent fd0b35d commit 876bba9

File tree

11 files changed

+297
-35
lines changed

11 files changed

+297
-35
lines changed

crates/bitcoind_rpc/examples/filter_iter.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ fn main() -> anyhow::Result<()> {
8989
}
9090
}
9191

92+
#[allow(deprecated)]
9293
for canon_tx in graph.graph().list_canonical_txs(
9394
&chain,
9495
chain.tip().block_id(),

crates/bitcoind_rpc/tests/test_emitter.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -647,6 +647,7 @@ fn test_expect_tx_evicted() -> anyhow::Result<()> {
647647
// Update graph with evicted tx.
648648
let _ = graph.batch_insert_relevant_evicted_at(mempool_event.evicted);
649649

650+
#[allow(deprecated)]
650651
let canonical_txids = graph
651652
.graph()
652653
.list_canonical_txs(&chain, chain_tip, CanonicalizationParams::default())

crates/chain/benches/canonicalization.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ fn setup<F: Fn(&mut KeychainTxGraph, &LocalChain)>(f: F) -> (KeychainTxGraph, Lo
9191
}
9292

9393
fn run_list_canonical_txs(tx_graph: &KeychainTxGraph, chain: &LocalChain, exp_txs: usize) {
94+
#[allow(deprecated)]
9495
let txs = tx_graph.graph().list_canonical_txs(
9596
chain,
9697
chain.tip().block_id(),
@@ -99,6 +100,15 @@ fn run_list_canonical_txs(tx_graph: &KeychainTxGraph, chain: &LocalChain, exp_tx
99100
assert_eq!(txs.count(), exp_txs);
100101
}
101102

103+
fn run_list_ordered_canonical_txs(tx_graph: &KeychainTxGraph, chain: &LocalChain, exp_txs: usize) {
104+
let txs = tx_graph.graph().list_ordered_canonical_txs(
105+
chain,
106+
chain.tip().block_id(),
107+
CanonicalizationParams::default(),
108+
);
109+
assert_eq!(txs.count(), exp_txs);
110+
}
111+
102112
fn run_filter_chain_txouts(tx_graph: &KeychainTxGraph, chain: &LocalChain, exp_txos: usize) {
103113
let utxos = tx_graph.graph().filter_chain_txouts(
104114
chain,
@@ -147,6 +157,13 @@ pub fn many_conflicting_unconfirmed(c: &mut Criterion) {
147157
let (tx_graph, chain) = (tx_graph.clone(), chain.clone());
148158
move |b| b.iter(|| run_list_canonical_txs(&tx_graph, &chain, 2))
149159
});
160+
c.bench_function(
161+
"many_conflicting_unconfirmed::list_ordered_canonical_txs",
162+
{
163+
let (tx_graph, chain) = (tx_graph.clone(), chain.clone());
164+
move |b| b.iter(|| run_list_ordered_canonical_txs(&tx_graph, &chain, 2))
165+
},
166+
);
150167
c.bench_function("many_conflicting_unconfirmed::filter_chain_txouts", {
151168
let (tx_graph, chain) = (tx_graph.clone(), chain.clone());
152169
move |b| b.iter(|| run_filter_chain_txouts(&tx_graph, &chain, 2))
@@ -185,6 +202,10 @@ pub fn many_chained_unconfirmed(c: &mut Criterion) {
185202
let (tx_graph, chain) = (tx_graph.clone(), chain.clone());
186203
move |b| b.iter(|| run_list_canonical_txs(&tx_graph, &chain, 2101))
187204
});
205+
c.bench_function("many_chained_unconfirmed::list_ordered_canonical_txs", {
206+
let (tx_graph, chain) = (tx_graph.clone(), chain.clone());
207+
move |b| b.iter(|| run_list_ordered_canonical_txs(&tx_graph, &chain, 2101))
208+
});
188209
c.bench_function("many_chained_unconfirmed::filter_chain_txouts", {
189210
let (tx_graph, chain) = (tx_graph.clone(), chain.clone());
190211
move |b| b.iter(|| run_filter_chain_txouts(&tx_graph, &chain, 1))
@@ -234,6 +255,13 @@ pub fn nested_conflicts(c: &mut Criterion) {
234255
let (tx_graph, chain) = (tx_graph.clone(), chain.clone());
235256
move |b| b.iter(|| run_list_canonical_txs(&tx_graph, &chain, GRAPH_DEPTH))
236257
});
258+
c.bench_function(
259+
"nested_conflicts_unconfirmed::list_ordered_canonical_txs",
260+
{
261+
let (tx_graph, chain) = (tx_graph.clone(), chain.clone());
262+
move |b| b.iter(|| run_list_ordered_canonical_txs(&tx_graph, &chain, GRAPH_DEPTH))
263+
},
264+
);
237265
c.bench_function("nested_conflicts_unconfirmed::filter_chain_txouts", {
238266
let (tx_graph, chain) = (tx_graph.clone(), chain.clone());
239267
move |b| b.iter(|| run_filter_chain_txouts(&tx_graph, &chain, GRAPH_DEPTH))

crates/chain/src/canonical_iter.rs

Lines changed: 187 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use crate::collections::{HashMap, HashSet, VecDeque};
2-
use crate::tx_graph::{TxAncestors, TxDescendants};
2+
use crate::tx_graph::{CanonicalTx, TxAncestors, TxDescendants};
33
use crate::{Anchor, ChainOracle, TxGraph};
44
use alloc::boxed::Box;
55
use alloc::collections::BTreeSet;
@@ -342,3 +342,189 @@ impl<A: Clone> CanonicalReason<A> {
342342
}
343343
}
344344
}
345+
346+
/// Iterator that yields transactions in topological order with proper sorting within levels.
347+
pub(crate) struct TopologicalIterator<'a, A> {
348+
/// Map of txid to its canonical transaction
349+
canonical_txs: HashMap<Txid, CanonicalTx<'a, Arc<Transaction>, A>>,
350+
351+
/// Current level of transactions to process
352+
current_level: Vec<Txid>,
353+
/// Next level of transactions to process
354+
next_level: Vec<Txid>,
355+
356+
/// Adjacency list: parent txid -> list of children txids
357+
children_map: HashMap<Txid, Vec<Txid>>,
358+
/// Number of unprocessed parents for each transaction
359+
parent_count: HashMap<Txid, usize>,
360+
361+
/// Current index in the current level
362+
current_index: usize,
363+
}
364+
365+
impl<'a, A: Clone + Anchor> TopologicalIterator<'a, A> {
366+
pub(crate) fn new(canonical_txs: Vec<CanonicalTx<'a, Arc<Transaction>, A>>) -> Self {
367+
// Build a map from txid to canonical tx for quick lookup
368+
let mut tx_map: HashMap<Txid, CanonicalTx<'a, Arc<Transaction>, A>> = HashMap::new();
369+
let mut canonical_set: HashSet<Txid> = HashSet::new();
370+
371+
for canonical_tx in canonical_txs {
372+
let txid = canonical_tx.tx_node.txid;
373+
canonical_set.insert(txid);
374+
tx_map.insert(txid, canonical_tx);
375+
}
376+
377+
// Build the dependency graph (txid -> parents it depends on)
378+
let mut dependencies: HashMap<Txid, Vec<Txid>> = HashMap::new();
379+
let mut has_parents: HashSet<Txid> = HashSet::new();
380+
381+
for &txid in canonical_set.iter() {
382+
let canonical_tx = tx_map.get(&txid).expect("txid must exist in map");
383+
let tx = &canonical_tx.tx_node.tx;
384+
385+
// Find all parents (transactions this one depends on)
386+
let mut parents = Vec::new();
387+
if !tx.is_coinbase() {
388+
for txin in &tx.input {
389+
let parent_txid = txin.previous_output.txid;
390+
// Only include if the parent is also canonical
391+
if canonical_set.contains(&parent_txid) {
392+
parents.push(parent_txid);
393+
has_parents.insert(txid);
394+
}
395+
}
396+
}
397+
398+
if !parents.is_empty() {
399+
dependencies.insert(txid, parents);
400+
}
401+
}
402+
403+
// Build adjacency list and parent counts for traversal
404+
let mut parent_count = HashMap::new();
405+
let mut children_map: HashMap<Txid, Vec<Txid>> = HashMap::new();
406+
407+
for (txid, parents) in &dependencies {
408+
for parent_txid in parents {
409+
children_map.entry(*parent_txid).or_default().push(*txid);
410+
*parent_count.entry(*txid).or_insert(0) += 1;
411+
}
412+
}
413+
414+
// Find root transactions (those with no parents in the canonical set)
415+
let roots: Vec<Txid> = canonical_set
416+
.iter()
417+
.filter(|&&txid| !has_parents.contains(&txid))
418+
.copied()
419+
.collect();
420+
421+
// Sort the initial level
422+
let mut current_level = roots;
423+
Self::sort_level_by_chain_position(&mut current_level, &tx_map);
424+
425+
Self {
426+
canonical_txs: tx_map,
427+
current_level,
428+
next_level: Vec::new(),
429+
children_map,
430+
parent_count,
431+
current_index: 0,
432+
}
433+
}
434+
435+
/// Sort transactions within a level by their chain position
436+
/// Confirmed transactions come first (sorted by height), then unconfirmed (sorted by last_seen)
437+
fn sort_level_by_chain_position(
438+
level: &mut [Txid],
439+
canonical_txs: &HashMap<Txid, CanonicalTx<'a, Arc<Transaction>, A>>,
440+
) {
441+
level.sort_by(|&a_txid, &b_txid| {
442+
let a_tx = canonical_txs.get(&a_txid).expect("txid must exist");
443+
let b_tx = canonical_txs.get(&b_txid).expect("txid must exist");
444+
445+
use crate::ChainPosition;
446+
use core::cmp::Ordering;
447+
448+
match (&a_tx.chain_position, &b_tx.chain_position) {
449+
// Both confirmed: sort by confirmation height
450+
(
451+
ChainPosition::Confirmed {
452+
anchor: a_anchor, ..
453+
},
454+
ChainPosition::Confirmed {
455+
anchor: b_anchor, ..
456+
},
457+
) => {
458+
let a_height = a_anchor.confirmation_height_upper_bound();
459+
let b_height = b_anchor.confirmation_height_upper_bound();
460+
a_height.cmp(&b_height)
461+
}
462+
// Confirmed comes before unconfirmed
463+
(ChainPosition::Confirmed { .. }, ChainPosition::Unconfirmed { .. }) => {
464+
Ordering::Less
465+
}
466+
// Unconfirmed comes after confirmed
467+
(ChainPosition::Unconfirmed { .. }, ChainPosition::Confirmed { .. }) => {
468+
Ordering::Greater
469+
}
470+
// Both unconfirmed: sort by first_seen (earlier timestamp first)
471+
(
472+
ChainPosition::Unconfirmed {
473+
first_seen: a_first_seen,
474+
..
475+
},
476+
ChainPosition::Unconfirmed {
477+
first_seen: b_first_seen,
478+
..
479+
},
480+
) => {
481+
// Earlier timestamps come first
482+
a_first_seen.cmp(b_first_seen)
483+
}
484+
}
485+
});
486+
}
487+
488+
fn advance_to_next_level(&mut self) {
489+
self.current_level = core::mem::take(&mut self.next_level);
490+
Self::sort_level_by_chain_position(&mut self.current_level, &self.canonical_txs);
491+
self.current_index = 0;
492+
}
493+
}
494+
495+
impl<'a, A: Clone + Anchor> Iterator for TopologicalIterator<'a, A> {
496+
type Item = CanonicalTx<'a, Arc<Transaction>, A>;
497+
498+
fn next(&mut self) -> Option<Self::Item> {
499+
// If we've exhausted the current level, move to next
500+
if self.current_index >= self.current_level.len() {
501+
if self.next_level.is_empty() {
502+
return None;
503+
}
504+
self.advance_to_next_level();
505+
}
506+
507+
let current_txid = self.current_level[self.current_index];
508+
self.current_index += 1;
509+
510+
// If this is the last item in current level, prepare dependents for next level
511+
if self.current_index == self.current_level.len() {
512+
// Process all dependents of all transactions in current level
513+
for &tx in &self.current_level {
514+
if let Some(children) = self.children_map.get(&tx) {
515+
for &child in children {
516+
if let Some(count) = self.parent_count.get_mut(&child) {
517+
*count -= 1;
518+
if *count == 0 {
519+
self.next_level.push(child);
520+
}
521+
}
522+
}
523+
}
524+
}
525+
}
526+
527+
// Return the CanonicalTx for the current txid
528+
self.canonical_txs.get(&current_txid).cloned()
529+
}
530+
}

crates/chain/src/tx_graph.rs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -996,6 +996,10 @@ impl<A: Anchor> TxGraph<A> {
996996
/// If the [`ChainOracle`] is infallible, [`list_canonical_txs`] can be used instead.
997997
///
998998
/// [`list_canonical_txs`]: Self::list_canonical_txs
999+
#[deprecated(
1000+
since = "0.23.3",
1001+
note = "Use `list_ordered_canonical_txs` instead, which returns transactions in topological order"
1002+
)]
9991003
pub fn try_list_canonical_txs<'a, C: ChainOracle + 'a>(
10001004
&'a self,
10011005
chain: &'a C,
@@ -1080,16 +1084,51 @@ impl<A: Anchor> TxGraph<A> {
10801084
/// This is the infallible version of [`try_list_canonical_txs`].
10811085
///
10821086
/// [`try_list_canonical_txs`]: Self::try_list_canonical_txs
1087+
#[deprecated(
1088+
since = "0.23.3",
1089+
note = "Use `list_ordered_canonical_txs` instead, which returns transactions in topological order"
1090+
)]
10831091
pub fn list_canonical_txs<'a, C: ChainOracle<Error = Infallible> + 'a>(
10841092
&'a self,
10851093
chain: &'a C,
10861094
chain_tip: BlockId,
10871095
params: CanonicalizationParams,
10881096
) -> impl Iterator<Item = CanonicalTx<'a, Arc<Transaction>, A>> {
1097+
#[allow(deprecated)]
10891098
self.try_list_canonical_txs(chain, chain_tip, params)
10901099
.map(|res| res.expect("infallible"))
10911100
}
10921101

1102+
/// List graph transactions that are in `chain` with `chain_tip` in topological order.
1103+
///
1104+
/// Each transaction is represented as a [`CanonicalTx`] that contains where the transaction is
1105+
/// observed in-chain, and the [`TxNode`].
1106+
///
1107+
/// Transactions are returned in topological spending order, meaning that if transaction B
1108+
/// spends from transaction A, then A will always appear before B in the resulting list.
1109+
///
1110+
/// This is the infallible version which uses [`list_canonical_txs`] internally and then
1111+
/// reorders the transactions based on their spending relationships.
1112+
///
1113+
/// [`list_canonical_txs`]: Self::list_canonical_txs
1114+
pub fn list_ordered_canonical_txs<'a, C: ChainOracle<Error = Infallible>>(
1115+
&'a self,
1116+
chain: &'a C,
1117+
chain_tip: BlockId,
1118+
params: CanonicalizationParams,
1119+
) -> impl Iterator<Item = CanonicalTx<'a, Arc<Transaction>, A>> {
1120+
use crate::canonical_iter::TopologicalIterator;
1121+
1122+
// First, get all canonical transactions
1123+
#[allow(deprecated)]
1124+
let canonical_txs: Vec<CanonicalTx<'a, Arc<Transaction>, A>> =
1125+
self.list_canonical_txs(chain, chain_tip, params).collect();
1126+
1127+
// Use the topological iterator to get the correct ordering
1128+
// The iterator handles all the graph building internally
1129+
TopologicalIterator::new(canonical_txs)
1130+
}
1131+
10931132
/// Get a filtered list of outputs from the given `outpoints` that are in `chain` with
10941133
/// `chain_tip`.
10951134
///
@@ -1118,6 +1157,7 @@ impl<A: Anchor> TxGraph<A> {
11181157
) -> Result<impl Iterator<Item = (OI, FullTxOut<A>)> + 'a, C::Error> {
11191158
let mut canon_txs = HashMap::<Txid, CanonicalTx<Arc<Transaction>, A>>::new();
11201159
let mut canon_spends = HashMap::<OutPoint, Txid>::new();
1160+
#[allow(deprecated)]
11211161
for r in self.try_list_canonical_txs(chain, chain_tip, params) {
11221162
let canonical_tx = r?;
11231163
let txid = canonical_tx.tx_node.txid;
@@ -1418,6 +1458,7 @@ impl<A: Anchor> TxGraph<A> {
14181458
I: fmt::Debug + Clone + Ord + 'a,
14191459
{
14201460
let indexer = indexer.as_ref();
1461+
#[allow(deprecated)]
14211462
self.try_list_canonical_txs(chain, chain_tip, CanonicalizationParams::default())
14221463
.flat_map(move |res| -> Vec<Result<(ScriptBuf, Txid), C::Error>> {
14231464
let range = &spk_index_range;

crates/chain/tests/test_indexed_tx_graph.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -782,6 +782,7 @@ fn test_get_chain_position() {
782782
}
783783

784784
// check chain position
785+
#[allow(deprecated)]
785786
let chain_pos = graph
786787
.graph()
787788
.list_canonical_txs(

0 commit comments

Comments
 (0)