-
Notifications
You must be signed in to change notification settings - Fork 307
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
feat(chain): Introduce tx_graph::Update
WIP
#1553
Conversation
@LagginTimes forgot to mention that this is step 1 for #1543 |
I would like to avoid unnecessary refactoring like this for the 1.0 beta milestone, see my comment in #1543. |
@notmandatory I suggested this change because I thought it was necessary for 1.0. Please refer to @LLFourn's comment here: #1543 (comment) |
9353641
to
9411021
Compare
crates/chain/src/tx_graph.rs
Outdated
pub struct Update<A> { | ||
whole_txs: HashMap<Txid, Arc<Transaction>>, | ||
partial_txs: HashMap<OutPoint, TxOut>, | ||
last_seen: HashMap<Txid, u64>, | ||
anchors: BTreeSet<(A, Txid)>, | ||
} |
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.
Remember that we will be moving Update
into a separate crate bdk_core
so we won't have access to private members.
Additionally, we don't want to provide floating outputs to our update logic when we already have the full tx in the same update struct.
With these two requirements in mind, let's see various solutions.
- Publicize all members. This is a bad idea, since we can't enforce our invariances.
- Enforce invariance when caller inserts data. When inserting full tx, rm all associated floating txouts of the tx's txid. Don't insert floating txout when there is a full tx of same txid.
- Enforce invariance when extracting data. When extracting full txs, also rm floating txouts with the same txid. When extracting floating txouts, ignore elements which have an associated full tx.
I think enforcing invariance when caller inserts data is easier to reason with.
170e8ba
to
6818d79
Compare
6818d79
to
576f31a
Compare
This is replaced by #1568 right? |
Replaced by #1568 |
Description
Introduces the
tx_graph::Update
struct, which will be used to update aTxGraph
. This will replace the old method of updating aTxGraph
with aTxGraph
.Notes to the reviewers
With the current implementation, there is some goofiness in the
esplora
/electrum
wallet examples andtest_electrum::sync_with_electrum()
, where anUpdate
is converted into aTxGraph
to executeupdate_last_seen_unconfirmed()
, before reverting theTxGraph
back into anUpdate
again. Implementing #1550 would prevent the need for this.Changelog notice
WIP
Checklists
All Submissions:
cargo fmt
andcargo clippy
before committing