refactor(chain)!: Remove Anchor trait and make anchors unique to (Txid, BlockId)#1515
refactor(chain)!: Remove Anchor trait and make anchors unique to (Txid, BlockId)#1515LagginTimes wants to merge 1 commit intobitcoindevkit:masterfrom
Anchor trait and make anchors unique to (Txid, BlockId)#1515Conversation
6afb3a2 to
9a926fe
Compare
c2f280f to
afe5511
Compare
notmandatory
left a comment
There was a problem hiding this comment.
Overall looks like a nice simplification making Anchor a concrete type, and replacing ConfirmationBlockTime with BlockTime. I primarily focused on changes in tx_data_traits.rs as the rest of the changes are downstream.
Once docs are done this looks good to me.
@evanlinjin is it OK to merge this one before #1514 ?
evanlinjin
left a comment
There was a problem hiding this comment.
This is an initial look. Thanks for pushing so much work out in short notice.
crates/chain/src/tx_graph.rs
Outdated
| pub struct TxGraph<AM = BlockTime> { | ||
| // all transactions that the graph is aware of in format: `(tx_node, tx_anchors)` | ||
| txs: HashMap<Txid, (TxNodeInternal, BTreeSet<A>)>, | ||
| txs: HashMap<Txid, (TxNodeInternal, BTreeMap<Anchor, AM>)>, |
There was a problem hiding this comment.
| txs: HashMap<Txid, (TxNodeInternal, BTreeMap<Anchor, AM>)>, | |
| txs: HashMap<Txid, (TxNodeInternal, BTreeSet<BlockId>)>, |
Anchor contains the txid. It does not make as part of the value of something keyed by txid.
Also we can lookup the A (anchor meta) from anchors.
crates/chain/src/tx_graph.rs
Outdated
| pub tx: T, | ||
| /// The blocks that the transaction is "anchored" in. | ||
| pub anchors: &'a BTreeSet<A>, | ||
| pub anchors: &'a BTreeMap<Anchor, AM>, |
There was a problem hiding this comment.
| pub anchors: &'a BTreeMap<Anchor, AM>, | |
| pub anchors: Range<'a, Anchor, AM>, |
I'm not sure if this will work? Getting a range from TxGraph::anchors.
|
@notmandatory I prefer we rebase this on top of #1514 since that PR is more "final" than this one. |
|
As discussed privately, it makes sense to rm pub struct TxGraph<AM = BlockTime> {
// all transactions that the graph is aware of in format: `(tx_node, tx_anchors)`
txs: HashMap<Txid, (TxNodeInternal, BTreeMap<BlockId, AM>)>,
spends: BTreeMap<OutPoint, HashSet<Txid>>,
last_seen: HashMap<Txid, u64>,
// 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>,
}See P.O.C: 63d9e6a |
afe5511 to
e04d221
Compare
The `Anchor` trait has been removed and anchors are now unique to (`Txid`, `BlockId`).
e04d221 to
8605283
Compare
|
This is great work. However, we have concluded that we do not have enough time to get this into |
|
I'll move this out of the alpha milestone, thanks guys. |
|
Technically wallet |
|
This is unfortunately so outdated that it probably is better to start from scratch. |
Resolves bitcoindevkit/bdk_wallet#90.
Description
The
Anchortrait has been removed and anchors are now unique to (Txid,BlockId).Notes to the reviewers
Documentation have been mostly untouched and need to be written/updated.
Changelog notice
Anchortrait has been removed.Anchortype andBlockTimestruct have been added tobdk_chain.Checklists
All Submissions:
cargo fmtandcargo clippybefore committing