-
Notifications
You must be signed in to change notification settings - Fork 417
Introduce evicted-at/last-evicted timestamps
#1839
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
Conversation
6dc26c4 to
0fcc097
Compare
540a83f to
5107b3a
Compare
5718a3e to
07903a8
Compare
evicted-at/last-evicted timestamps
07903a8 to
9d6c795
Compare
This is a set of evicted txs from the mempool.
0f10b55 to
806a7d0
Compare
The evicted-at and last-evicted timestamp informs the `TxGraph` when the transaction was last deemed as missing (evicted) from the mempool. The canonicalization algorithm is changed to disregard transactions with a last-evicted timestamp greater or equal to it's last-seen timestamp. The exception is when we have a canonical descendant due to rules of transitivity. Update rusqlite_impl to persist `last_evicted`. Also update docs: * Remove duplicate paragraphs about `ChangeSet`s. * Add "Canonicalization" section which expands on methods that require canonicalization and the associated data types used in the canonicalization algorithm.
The spk history returned from Electrum should have these txs present. Any missing tx will be considered evicted from the mempool.
* `TxGraph::try_list_expected_spk_txids` * `TxGraph::list_expected_spk_txids` * `IndexedTxGraph::try_list_expected_spk_txids` * `IndexedTxGraph::list_expected_spk_txids` Co-authored-by: valued mammal <[email protected]> Co-authored-by: Wei Chen <[email protected]>
Co-authored-by: Wei Chen <[email protected]>
Also add `detect_receive_tx_cancel` test. Also rm `miniscript` dependency. Update ci to rm `miniscript/no-std` for `bdk_esplora`. Co-authored-by: Wei Chen <[email protected]>
Methods `next_spk` and `iter_spks` are removed. These are superceded with `next_spk_with_expected_txids` and `iter_spks_with_expected_txids`.
The tests that we introduce in this PR are based on @ErikDeSmedt's tests (but we target for esplora/electrum in our PR here). This PR does not fix evicted-tx-detection for So I think it is okay to merge this without Erik's confirmation. |
LLFourn
left a comment
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.
ACK 0a20724
LagginTimes
left a comment
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.
One last nit that I saw.
| /// | ||
| /// This is used to fill [`SyncRequestBuilder::expected_spk_txids`](bdk_core::spk_client::SyncRequestBuilder::expected_spk_txids). | ||
| /// | ||
| /// The spk index range can be contrained with `range`. |
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.
| /// The spk index range can be contrained with `range`. | |
| /// The spk index range can be constrained with `range`. |
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.
I wonder how easy it would be to set up CI to check for spelling mistakes. Might save time.
|
ACK 0a20724 |
Partially Fixes #1740.
Replaces #1765.
Replaces #1811.
Description
This PR allows the receiving structures (
bdk_chain,bdk_wallet) to detect and evict incoming transactions that are double spent (cancelled).We add a new field to
TxUpdate(TxUpdate::evicted_ats), which in turn, updates thelast_evictedtimestamps that are tracked/persisted byTxGraph. This is similar to howTxUpdate::seen_atsupdate thelast_seentimestamp inTxGraph. Transactions with alast_evictedtimestamp higher than theirlast_seentimestamp are considered evicted.SpkWithExpectedTxidsis introduced inSpkClientto track expectedTxids for eachspk. During a sync, if anyTxids fromSpkWithExpectedTxidsare not in the current history of anspkobtained from the chain source, thoseTxids are considered missing. Support forSpkWithExpectedTxidshas been added to bothbdk_electrumandbdk_esplorachain source crates.The canonicalization algorithm is updated to disregard transactions with a
last_evictedtimestamp greater than or equal to theirlast_seentimestamp, except in cases where transitivity rules apply.Notes to the reviewers
This PR does not fix #1740 for block-by-block chain source (such as
bdk_bitcoind_rpc). This work is done in a separate PR (#1857).Changelog notice
TxUpdate::evicted_atswhich tracks transactions that have been replaced and are no longer present in mempool.TxGraphto tracklast_evictedtimestamps. This is when a transaction is last marked as missing from the mempool.last_evictedtimestamp greater than or equal to it'slast_seentimestamp, except when a canonical descendant exists due to rules of transitivity.SpkWithExpectedTxidsinspk_clientwhich keeps track of expectedTxids for eachspk.bdk_electrumandbdk_esplorato understandSpkWithExpectedTxids.SyncRequestBuilder::expected_txids_of_spkmethod which adds an association betweentxids andspks.Checklists
All Submissions:
cargo fmtandcargo clippybefore committingNew Features:
Bugfixes: