Introduce broadcast queue.#220
Closed
evanlinjin wants to merge 3 commits intobitcoindevkit:masterfrom
Closed
Conversation
- `apply_update` no longer requires std - Removed `apply_update_at` - `start_sync_with_revealed_spks` is now behind "std" feature - Added `start_sync_at` - `start_sync_*` includes the expected spk history - Add example test for mempool eviction, includes persistence - Update bdk_file_store, which uses the latest from bdk_core - `start_full_scan` requires std feature Note that `tx_graph::ChangeSet` gained a new default-able field `last_evicted` to account for evictions.
- Improve docs for `test_utils::insert_tx`
3dbd1b4 to
5b15213
Compare
5b15213 to
0b05917
Compare
Pull Request Test Coverage Report for Build 14664394129Details
💛 - Coveralls |
8 tasks
Member
Author
To Do (as discussed)
In the future, if users are requesting that we expose |
Member
|
@evanlinjin is it be possible to create a version of this feature, possibly without persistence changes, in a non-breaking way for the |
Member
Author
|
@notmandatory I don't think this can be done in a non-breaking way, as the only way something like this would be useful is if we persist the data (thus modifying the changeset). I could finish this PR soon though |
Member
Author
|
Replaced by #257 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #40
Description
This PR builts on top of #208 and bitcoindevkit/bdk#1808.
The goal is to evaluate whether the changes introduced in bitcoindevkit/bdk#1808 provide a sufficient foundation to implement and maintain a broadcast queue within
bdk_wallet.Requirements:
Notes to the reviewers
Design Rationale
For getter-methods on
Walletthat requires canonicalization internally, I've opted to pass inCanonicalizationParamsas an input. The reasoning is it's up to the application to decide whether or not to include unbroadcasted txs in their UTXO-set/tx-list. There is a methodWallet::include_unbroadcasted()which returns aCanonicalizationParamswhich would assume all unbroadcasted as canonical.The alternative would be to pass in a boolean
include_unbroadcastedbut I felt like it would be more brittle.Where to start reviewing
The unbroadcasted queue in introduced in a new file
wallet/unbroadcasted.rs. Other than that, the majority of the changes are inwallet/mod.rs.Changelog notice
WIP
Checklists
All Submissions:
cargo fmtandcargo clippybefore committingNew Features:
Bugfixes: