bdk
bdk copied to clipboard
[chain] Consider Merging IndexedTxGraph and TxGraph into one thing
currently:
#[derive(Debug)]
pub struct IndexedTxGraph<A, I> {
/// Transaction index.
pub index: I,
graph: TxGraph<A>,
}
But it could also just be:
#[derive(Clone, Debug, PartialEq)]
pub struct TxGraph<A = (), I = ()> {
// all transactions that the graph is aware of in format: `(tx_node, tx_anchors, tx_last_seen)`
txs: HashMap<Txid, (TxNodeInternal, BTreeSet<A>, u64)>,
spends: BTreeMap<OutPoint, HashSet<Txid>>,
anchors: BTreeSet<(A, Txid)>,
// This atrocity exists so that `TxGraph::outspends()` can return a reference.
// FIXME: This can be removed once `HashSet::new` is a const fn.
empty_outspends: HashSet<Txid>,
pub index: I
}
But we would have to give up the derive PartialEq
on TxGraph
because I don't think we want to force indexes to implement it.
The motivation is avoiding having to duplicate every mutating API on TxGraph
to IndexedTxGraph
. We seem to have some trouble doing this. For example, IndexedTxGraph
and TxGraph
both have insert_tx
but they have different arguments atm but they should be the same. Also in #1041 I a method is being added to IndexedTxGraph
but it should also be on TxGraph
but the PR author forgot.
The downside is that TxGraph
would not longer be a pure data structure but since we don't use it like that I don't think this matters.
"the PR author"
I moved this to alpha.4 since we want to get any functional changes in before a beta release.
FWIW I think this is still a good idea.
Can we postpone it to a 1.1 version @notmandatory?
I think it is okay to postpone this to after 1.0.0. I think it may simplify the API and make it cleaner to use with less foot-guns. However, it is also not critical to have this.