You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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
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:
Creating a new tx should not replace unbroadcasted txs that were already in the wallet.
The broadcast queue should support transaction replacements, with the most recently inserted version taking precedence.
Notes to the reviewers
Design Rationale
For getter-methods on Wallet that requires canonicalization internally, I've opted to pass in CanonicalizationParams as 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 method Wallet::include_unbroadcasted() which returns a CanonicalizationParams which would assume all unbroadcasted as canonical.
The alternative would be to pass in a boolean include_unbroadcasted but 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 in wallet/mod.rs.
- `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.
@evanlinjin is it be possible to create a version of this feature, possibly without persistence changes, in a non-breaking way for the bdk_wallet2.1.0 release? Or is there some other way to fix #166 with the current bdk_wallet + bdk_chain ?
@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
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
Wallet
that requires canonicalization internally, I've opted to pass inCanonicalizationParams
as 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 aCanonicalizationParams
which would assume all unbroadcasted as canonical.The alternative would be to pass in a boolean
include_unbroadcasted
but 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 fmt
andcargo clippy
before committingNew Features:
Bugfixes: