Skip to content

Commit c2a266d

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 376eb70 commit c2a266d

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
@@ -85,6 +85,7 @@ fn main() -> anyhow::Result<()> {
8585
}
8686
}
8787

88+
#[allow(deprecated)]
8889
for canon_tx in graph.graph().list_canonical_txs(
8990
&chain,
9091
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
@@ -635,6 +635,7 @@ fn test_expect_tx_evicted() -> anyhow::Result<()> {
635635
// Update graph with evicted tx.
636636
let _ = graph.batch_insert_relevant_evicted_at(mempool_event.evicted);
637637

638+
#[allow(deprecated)]
638639
let canonical_txids = graph
639640
.graph()
640641
.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
@@ -95,6 +95,7 @@ fn setup<F: Fn(&mut KeychainTxGraph, &LocalChain)>(f: F) -> (KeychainTxGraph, Lo
9595
}
9696

9797
fn run_list_canonical_txs(tx_graph: &KeychainTxGraph, chain: &LocalChain, exp_txs: usize) {
98+
#[allow(deprecated)]
9899
let txs = tx_graph.graph().list_canonical_txs(
99100
chain,
100101
chain.tip().block_id(),
@@ -103,6 +104,15 @@ fn run_list_canonical_txs(tx_graph: &KeychainTxGraph, chain: &LocalChain, exp_tx
103104
assert_eq!(txs.count(), exp_txs);
104105
}
105106

107+
fn run_list_ordered_canonical_txs(tx_graph: &KeychainTxGraph, chain: &LocalChain, exp_txs: usize) {
108+
let txs = tx_graph.graph().list_ordered_canonical_txs(
109+
chain,
110+
chain.tip().block_id(),
111+
CanonicalizationParams::default(),
112+
);
113+
assert_eq!(txs.count(), exp_txs);
114+
}
115+
106116
fn run_filter_chain_txouts(tx_graph: &KeychainTxGraph, chain: &LocalChain, exp_txos: usize) {
107117
let utxos = tx_graph.graph().filter_chain_txouts(
108118
chain,
@@ -151,6 +161,13 @@ pub fn many_conflicting_unconfirmed(c: &mut Criterion) {
151161
let (tx_graph, chain) = (tx_graph.clone(), chain.clone());
152162
move |b| b.iter(|| run_list_canonical_txs(&tx_graph, &chain, 2))
153163
});
164+
c.bench_function(
165+
"many_conflicting_unconfirmed::list_ordered_canonical_txs",
166+
{
167+
let (tx_graph, chain) = (tx_graph.clone(), chain.clone());
168+
move |b| b.iter(|| run_list_ordered_canonical_txs(&tx_graph, &chain, 2))
169+
},
170+
);
154171
c.bench_function("many_conflicting_unconfirmed::filter_chain_txouts", {
155172
let (tx_graph, chain) = (tx_graph.clone(), chain.clone());
156173
move |b| b.iter(|| run_filter_chain_txouts(&tx_graph, &chain, 2))
@@ -189,6 +206,10 @@ pub fn many_chained_unconfirmed(c: &mut Criterion) {
189206
let (tx_graph, chain) = (tx_graph.clone(), chain.clone());
190207
move |b| b.iter(|| run_list_canonical_txs(&tx_graph, &chain, 2101))
191208
});
209+
c.bench_function("many_chained_unconfirmed::list_ordered_canonical_txs", {
210+
let (tx_graph, chain) = (tx_graph.clone(), chain.clone());
211+
move |b| b.iter(|| run_list_ordered_canonical_txs(&tx_graph, &chain, 2101))
212+
});
192213
c.bench_function("many_chained_unconfirmed::filter_chain_txouts", {
193214
let (tx_graph, chain) = (tx_graph.clone(), chain.clone());
194215
move |b| b.iter(|| run_filter_chain_txouts(&tx_graph, &chain, 1))
@@ -238,6 +259,13 @@ pub fn nested_conflicts(c: &mut Criterion) {
238259
let (tx_graph, chain) = (tx_graph.clone(), chain.clone());
239260
move |b| b.iter(|| run_list_canonical_txs(&tx_graph, &chain, GRAPH_DEPTH))
240261
});
262+
c.bench_function(
263+
"nested_conflicts_unconfirmed::list_ordered_canonical_txs",
264+
{
265+
let (tx_graph, chain) = (tx_graph.clone(), chain.clone());
266+
move |b| b.iter(|| run_list_ordered_canonical_txs(&tx_graph, &chain, GRAPH_DEPTH))
267+
},
268+
);
241269
c.bench_function("nested_conflicts_unconfirmed::filter_chain_txouts", {
242270
let (tx_graph, chain) = (tx_graph.clone(), chain.clone());
243271
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
@@ -993,6 +993,10 @@ impl<A: Anchor> TxGraph<A> {
993993
/// If the [`ChainOracle`] is infallible, [`list_canonical_txs`] can be used instead.
994994
///
995995
/// [`list_canonical_txs`]: Self::list_canonical_txs
996+
#[deprecated(
997+
since = "0.23.3",
998+
note = "Use `list_ordered_canonical_txs` instead, which returns transactions in topological order"
999+
)]
9961000
pub fn try_list_canonical_txs<'a, C: ChainOracle + 'a>(
9971001
&'a self,
9981002
chain: &'a C,
@@ -1077,16 +1081,51 @@ impl<A: Anchor> TxGraph<A> {
10771081
/// This is the infallible version of [`try_list_canonical_txs`].
10781082
///
10791083
/// [`try_list_canonical_txs`]: Self::try_list_canonical_txs
1084+
#[deprecated(
1085+
since = "0.23.3",
1086+
note = "Use `list_ordered_canonical_txs` instead, which returns transactions in topological order"
1087+
)]
10801088
pub fn list_canonical_txs<'a, C: ChainOracle<Error = Infallible> + 'a>(
10811089
&'a self,
10821090
chain: &'a C,
10831091
chain_tip: BlockId,
10841092
params: CanonicalizationParams,
10851093
) -> impl Iterator<Item = CanonicalTx<'a, Arc<Transaction>, A>> {
1094+
#[allow(deprecated)]
10861095
self.try_list_canonical_txs(chain, chain_tip, params)
10871096
.map(|res| res.expect("infallible"))
10881097
}
10891098

1099+
/// List graph transactions that are in `chain` with `chain_tip` in topological order.
1100+
///
1101+
/// Each transaction is represented as a [`CanonicalTx`] that contains where the transaction is
1102+
/// observed in-chain, and the [`TxNode`].
1103+
///
1104+
/// Transactions are returned in topological spending order, meaning that if transaction B
1105+
/// spends from transaction A, then A will always appear before B in the resulting list.
1106+
///
1107+
/// This is the infallible version which uses [`list_canonical_txs`] internally and then
1108+
/// reorders the transactions based on their spending relationships.
1109+
///
1110+
/// [`list_canonical_txs`]: Self::list_canonical_txs
1111+
pub fn list_ordered_canonical_txs<'a, C: ChainOracle<Error = Infallible>>(
1112+
&'a self,
1113+
chain: &'a C,
1114+
chain_tip: BlockId,
1115+
params: CanonicalizationParams,
1116+
) -> impl Iterator<Item = CanonicalTx<'a, Arc<Transaction>, A>> {
1117+
use crate::canonical_iter::TopologicalIterator;
1118+
1119+
// First, get all canonical transactions
1120+
#[allow(deprecated)]
1121+
let canonical_txs: Vec<CanonicalTx<'a, Arc<Transaction>, A>> =
1122+
self.list_canonical_txs(chain, chain_tip, params).collect();
1123+
1124+
// Use the topological iterator to get the correct ordering
1125+
// The iterator handles all the graph building internally
1126+
TopologicalIterator::new(canonical_txs)
1127+
}
1128+
10901129
/// Get a filtered list of outputs from the given `outpoints` that are in `chain` with
10911130
/// `chain_tip`.
10921131
///
@@ -1115,6 +1154,7 @@ impl<A: Anchor> TxGraph<A> {
11151154
) -> Result<impl Iterator<Item = (OI, FullTxOut<A>)> + 'a, C::Error> {
11161155
let mut canon_txs = HashMap::<Txid, CanonicalTx<Arc<Transaction>, A>>::new();
11171156
let mut canon_spends = HashMap::<OutPoint, Txid>::new();
1157+
#[allow(deprecated)]
11181158
for r in self.try_list_canonical_txs(chain, chain_tip, params) {
11191159
let canonical_tx = r?;
11201160
let txid = canonical_tx.tx_node.txid;
@@ -1415,6 +1455,7 @@ impl<A: Anchor> TxGraph<A> {
14151455
I: fmt::Debug + Clone + Ord + 'a,
14161456
{
14171457
let indexer = indexer.as_ref();
1458+
#[allow(deprecated)]
14181459
self.try_list_canonical_txs(chain, chain_tip, CanonicalizationParams::default())
14191460
.flat_map(move |res| -> Vec<Result<(ScriptBuf, Txid), C::Error>> {
14201461
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
@@ -788,6 +788,7 @@ fn test_get_chain_position() {
788788
}
789789

790790
// check chain position
791+
#[allow(deprecated)]
791792
let chain_pos = graph
792793
.graph()
793794
.list_canonical_txs(

0 commit comments

Comments
 (0)