Introduce universal sync/full-scan structures for spk-based syncing#1413
Introduce universal sync/full-scan structures for spk-based syncing#1413evanlinjin merged 4 commits intobitcoindevkit:masterfrom
Conversation
b336db8 to
08edd58
Compare
These structures allows spk-based chain-sources to have a universal API. Co-authored-by: Steve Myers <steve@notmandatory.org>
* Changed `Wallet::apply_update` to also take in anything that implements `Into<Update>`. This allows us to directly apply a `FullScanResult` or `SyncResult`. * Added `start_full_scan` and `start_sync_with_revealed_spks` methods to `Wallet`. Co-authored-by: Steve Myers <steve@notmandatory.org>
| .full_scan(prev_tip, keychain_spks, STOP_GAP, PARALLEL_REQUESTS) | ||
| .full_scan(request, STOP_GAP, PARALLEL_REQUESTS) | ||
| .await?; | ||
| let now = std::time::UNIX_EPOCH.elapsed().unwrap().as_secs(); | ||
| let _ = update.tx_graph.update_last_seen_unconfirmed(now); | ||
| let _ = update.graph_update.update_last_seen_unconfirmed(now); |
There was a problem hiding this comment.
Great! I was going through this example yesterday and I was having tons of issues because even without this PR it was un-reproducible code.
Thanks!
There was a problem hiding this comment.
I'm sorry I don't quite understand?
storopoli
left a comment
There was a problem hiding this comment.
Concept ACK.
These are great changes.
I imagine that you are planning to implement this to electrum as well? Either in this PR or in a separate PR?
This allows the caller to track sync progress.
notmandatory
left a comment
There was a problem hiding this comment.
tACK c0374a0
Thanks for the improvements!
The examples ran fine. But I did have to change the wallet examples to use signet and the signet esplora server http://signet.bitcoindevkit.net since others are rate limiting and so the scans were failing with 429 errors.
| let mut visited = 0; | ||
| move |_| { | ||
| visited += 1; | ||
| eprintln!(" [ {:>6.2}% ]", (visited * 100) as f32 / total_spks as f32) |
There was a problem hiding this comment.
😃 nice touch showing progress as percents.
|
|
||
| /// Set the [`Script`]s that will be synced against. | ||
| /// | ||
| /// This consumes the [`SyncRequest`] and returns the updated one. |
There was a problem hiding this comment.
😃 I like how you changed all the Sync/FullScan Request functions to consume and return Self, makes them much easier to use.
b45897e feat(electrum): update docs and simplify logic of `ElectrumExt` (志宇) 92fb6cb chore(electrum): do not use `anyhow::Result` directly (志宇) b2f3cac feat(electrum): include option for previous `TxOut`s for fee calculation (Wei Chen) c0d7d60 feat(chain)!: use custom return types for `ElectrumExt` methods (志宇) 2945c6b fix(electrum): fixed `sync` functionality (Wei Chen) 9ed33c2 docs(electrum): fixed `full_scan`, `sync`, and crate documentation (Wei Chen) b1f861b feat: update logging of electrum examples (志宇) a6fdfb2 feat(electrum)!: use new sync/full-scan structs for `ElectrumExt` (志宇) 653e4fe feat(wallet): cache txs when constructing full-scan/sync requests (志宇) 58f27b3 feat(chain): introduce `TxCache` to `SyncRequest` and `FullScanRequest` (志宇) 721bb7f fix(chain): Make `Anchor` type in `FullScanResult` generic (志宇) e3cfb84 feat(chain): `TxGraph::insert_tx` reuses `Arc` (志宇) 2ffb656 refactor(electrum): remove `RelevantTxids` and track txs in `TxGraph` (Wei Chen) Pull request description: Fixes #1265 Possibly fixes #1419 ### Context Previous changes such as * Universal structures for full-scan/sync (PR #1413) * Making `CheckPoint` linked list query-able (PR #1369) * Making `Transaction`s cheaply-clonable (PR #1373) has allowed us to simplify the interaction between chain-source and receiving-structures (`bdk_chain`). The motivation is to accomplish something like this ([as mentioned here](#1153 (comment))): ```rust let things_I_am_interested_in = wallet.lock().unwrap().start_sync(); let update = electrum_or_esplora.sync(things_i_am_interested_in)?; wallet.lock().unwrap().apply_update(update)?: ``` ### Description This PR greatly simplifies the API of our Electrum chain-source (`bdk_electrum`) by making use of the aforementioned changes. Instead of referring back to the receiving `TxGraph` mid-sync/scan to determine which full transaction to fetch, we provide the Electrum chain-source already-fetched full transactions to start sync/scan (this is cheap, as transactions are wrapped in `Arc`s since #1373). In addition, an option has been added to include the previous `TxOut` for transactions received from an external wallet for fee calculation. ### Changelog notice * Change `TxGraph::insert_tx` to take in anything that satisfies `Into<Arc<Transaction>>`. This allows us to reuse the `Arc` pointer of what is being inserted. * Add `tx_cache` field to `SyncRequest` and `FullScanRequest`. * Make `Anchor` type in `FullScanResult` generic for more flexibility. * Change `ElectrumExt` methods to take in `SyncRequest`/`FullScanRequest` and return `SyncResult`/`FullScanResult`. Also update electrum examples accordingly. * Add `ElectrumResultExt` trait which allows us to convert the update `TxGraph` of `SyncResult`/`FullScanResult` for `bdk_electrum`. * Added an option for `full_scan` and `sync` to also fetch previous `TxOut`s to allow for fee calculation. ### Checklists #### All Submissions: * [x] I've signed all my commits * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md) * [x] I ran `cargo fmt` and `cargo clippy` before committing #### New Features: * [x] I've added tests for the new feature * [x] I've added docs for the new feature ACKs for top commit: ValuedMammal: ACK b45897e notmandatory: ACK b45897e Tree-SHA512: 1e274546015e7c7257965b36079ffe0cb3c2c0b7c2e0c322bcf32a06925a0c3e1119da1c8fd5318f1dbd82c2e952f6a07f227a9b023c48f506a62c93045d96d3
Fixes #1153
Replaces #1194
Description
Introduce universal structures that represent sync/full-scan requests/results.
Notes to the reviewers
This is based on #1194 but is different in the following ways:
SyncRequestandFullScanRequestis simplified and fields are exposed for more flexibility.Changelog notice
bdk_esplorachain-source to make use of new universal sync/full-scan structures.Checklists
All Submissions:
cargo fmtandcargo clippybefore committingNew Features: