-
Notifications
You must be signed in to change notification settings - Fork 458
refactor(chain): add CanonicalView topological ordering
#2219
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -169,7 +169,7 @@ impl<A: Anchor> CanonicalTxOut<ChainPosition<A>> { | |
|
|
||
| /// Canonical set of transactions from a [`TxGraph`]. | ||
| /// | ||
| /// `Canonical` provides a conflict-resolved list of transactions. It determines | ||
| /// `Canonical` provides an ordered, conflict-resolved set of transactions. It determines | ||
| /// which transactions are canonical (non-conflicted) based on the current chain state and | ||
| /// provides methods to query transaction data, unspent outputs, and balances. | ||
| /// | ||
|
|
@@ -179,14 +179,14 @@ impl<A: Anchor> CanonicalTxOut<ChainPosition<A>> { | |
| /// [`CanonicalTxs`]) | ||
| /// | ||
| /// The view maintains: | ||
| /// - A list of canonical transactions | ||
| /// - An ordered list of canonical transactions in topological-spending order | ||
| /// - A mapping of outpoints to the transactions that spend them | ||
| /// - The chain tip used for canonicalization | ||
| /// | ||
| /// [`TxGraph`]: crate::TxGraph | ||
| #[derive(Debug)] | ||
| pub struct Canonical<A, P> { | ||
| /// List of canonical transaction IDs. | ||
| /// Ordered list of transaction IDs in topological-spending order. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is only true for
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I was thinking about this. I don't think returning the topological order in here is the best approach, as this field is shared by both canonicalization and view tasks. As it's cheap to get the topological order, and it can be optional for the user, we can have it separated instead of in a field, the field being only the list of canonical_txs, which is common on both phases. |
||
| pub(crate) order: Vec<Txid>, | ||
| /// Map of transaction IDs to their transaction data and position. | ||
| pub(crate) txs: HashMap<Txid, (Arc<Transaction>, P)>, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| //! Phase 2 task: resolves canonical reasons into chain positions. | ||
|
|
||
| use crate::canonical_task::{CanonicalReason, ObservedIn}; | ||
| use crate::collections::{HashMap, VecDeque}; | ||
| use crate::collections::{HashMap, HashSet, VecDeque}; | ||
| use crate::tx_graph::TxDescendants; | ||
| use alloc::collections::BTreeSet; | ||
| use alloc::sync::Arc; | ||
|
|
@@ -88,6 +88,81 @@ impl<'g, A: Anchor> CanonicalViewTask<'g, A> { | |
| } | ||
| } | ||
|
|
||
| /// Topologically sort [`Txid`]s (parents before children) using Kahn's algorithm. | ||
| /// | ||
| /// Given a set of canonical transactions and their spending relationships, | ||
| /// returns a new ordering where for every spending relationship A -> B | ||
| /// (where B spends an output of A), A appears before B. | ||
| /// | ||
| /// The algorithm works in three phases: | ||
| /// | ||
| /// 1. **Build the dependency graph**: Using the `spends` map, derive parent->child edges. Each | ||
| /// `(outpoint, child_txid)` entry means `outpoint.txid` (the parent) must come before | ||
| /// `child_txid`. Only edges where both parent and child are in the canonical set are considered. | ||
| /// | ||
| /// 2. **Find roots**: [`Txid`]s with `in_degree == 0` have no canonical parents and can appear | ||
| /// first. These seed the processing queue. | ||
| /// | ||
| /// 3. **BFS traversal**: Dequeue a [`Txid`], append it to the result, and decrement the `in_degree` | ||
| /// of each of its children. Whenever a child reaches `in_degree == 0`, all its parents have been | ||
| /// placed, so it is enqueued. | ||
| /// | ||
| /// # Note | ||
| /// | ||
| /// The relative order among unrelated transactions (those with no spending | ||
| /// relationship) is not guaranteed to be deterministic across runs, since | ||
| /// it depends on `HashMap` iteration order. | ||
| fn sort_topologically(order: &[Txid], spends: &HashMap<OutPoint, Txid>) -> Vec<Txid> { | ||
| // Set of canonical txids — we only consider parent→child edges where | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use only ASCII here: |
||
| // both sides are in this set. | ||
| let canonical_set: HashSet<Txid> = order.iter().copied().collect(); | ||
|
|
||
| // in_degree[txid] tracks how many canonical parents this tx spends from. | ||
| // A tx with in_degree 0 has no canonical parents and can appear first. | ||
| let mut in_degree: HashMap<Txid, usize> = order.iter().map(|&txid| (txid, 0)).collect(); | ||
|
|
||
| // Adjacency list: maps each parent txid to the children that spend its | ||
| // outputs. Derived from `spends` where each entry (outpoint → child_txid) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use only ASCII here. Remove |
||
| // gives us an edge from outpoint.txid (parent) to child_txid. | ||
| let mut children: HashMap<Txid, Vec<Txid>> = HashMap::new(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The use of // Demonstrates the topological-order invariant on a graph where a child transaction has two
// inputs both spending from the same parent.
// a0 ( a0 )
// / \ ( | )
// / \ ( | )
// a0:0 a0:1 ( | )
// \ / ( | )
// \/ ( | )
// b0 ( b0 ) -> assumed canonical
Scenario {
name: "b0 spends a0:0 and a0:1, and both do not have anchors or last_seen",
tx_templates: &[
TxTemplate {
tx_name: "a0",
inputs: &[TxInTemplate::Bogus],
outputs: &[
TxOutTemplate::new(10_000, None),
TxOutTemplate::new(10_000, None),
],
anchors: &[],
last_seen: None,
assume_canonical: false,
},
TxTemplate {
tx_name: "b0",
inputs: &[TxInTemplate::PrevTx("a0", 0), TxInTemplate::PrevTx("a0", 1)],
outputs: &[TxOutTemplate::new(18_000, None)],
anchors: &[],
last_seen: None,
assume_canonical: true,
},
],
exp_chain_txs: Vec::from(["a0", "b0"]),
},
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I lean to use a
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, I'll take a look into that. |
||
|
|
||
| for (outpoint, &child) in spends { | ||
| let parent = outpoint.txid; | ||
| // Only consider edges where the parent is also canonical — spending | ||
| // from a tx outside this canonical set is not a dependency. | ||
| if canonical_set.contains(&parent) { | ||
| children.entry(parent).or_default().push(child); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we somehow push this child into an ordered position? I.e. sort the direct children by confirmation status followed by txid. This will make the order more correct and deterministic.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lexicographic order by |
||
| *in_degree.entry(child).or_default() += 1; | ||
| } | ||
| } | ||
|
|
||
| // Seed the queue with root transactions (those with no canonical parents). | ||
| let mut queue: VecDeque<Txid> = in_degree | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this where sorted lexicographically by |
||
| .iter() | ||
| .filter(|(_, &d)| d == 0) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we do
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, though I will change it from degree to inputs or other better terminology. |
||
| .map(|(&txid, _)| txid) | ||
| .collect(); | ||
|
|
||
| // Process the queue: each time we "remove" a tx from the graph, we | ||
| // decrement the in_degree of its children. When a child reaches | ||
| // in_degree 0, all its parents have been placed so it is ready. | ||
| let mut sorted = Vec::with_capacity(order.len()); | ||
| while let Some(txid) = queue.pop_front() { | ||
| sorted.push(txid); | ||
| if let Some(deps) = children.get(&txid) { | ||
| for &child in deps { | ||
| let d = in_degree.get_mut(&child).unwrap(); | ||
| *d -= 1; | ||
| if *d == 0 { | ||
| queue.push_back(child); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| sorted | ||
| } | ||
|
|
||
| impl<'g, A: Anchor> ChainQuery for CanonicalViewTask<'g, A> { | ||
| type Output = CanonicalView<A>; | ||
|
|
||
|
|
@@ -218,6 +293,7 @@ impl<'g, A: Anchor> ChainQuery for CanonicalViewTask<'g, A> { | |
| } | ||
| } | ||
|
|
||
| let view_order = sort_topologically(&view_order, &self.spends); | ||
| CanonicalView::new(self.tip, view_order, view_txs, self.spends) | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you choose to use
setinstead oflist?listis closer to ordered sequence than set.