Introduce universal sync/full-scan structures for spk-based syncing #1413

evanlinjin · 2024-04-25T02:49:46Z

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:

The functionality to print scan/sync progress is not reduced.
SyncRequest and FullScanRequest is simplified and fields are exposed for more flexibility.

Changelog notice

Add universal structures for initiating/receiving sync/full-scan requests/results for spk-based syncing.
Updated bdk_esplora chain-source to make use of new universal sync/full-scan structures.

Checklists

All Submissions:

I've signed all my commits
I followed the contribution guidelines
I ran cargo fmt and cargo clippy before committing

New Features:

I've added tests for the new feature
I've added docs for the new feature

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>

storopoli · 2024-04-26T09:34:07Z

example-crates/wallet_esplora_async/src/main.rs

-        .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);


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!

I'm sorry I don't quite understand?

storopoli

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

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.

notmandatory · 2024-04-29T00:09:57Z

example-crates/example_esplora/src/main.rs

+                    let mut visited = 0;
+                    move |_| {
+                        visited += 1;
+                        eprintln!(" [ {:>6.2}% ]", (visited * 100) as f32 / total_spks as f32)


😃 nice touch showing progress as percents.

notmandatory · 2024-04-29T00:14:40Z

crates/chain/src/spk_client.rs

+
+    /// Set the [`Script`]s that will be synced against.
+    ///
+    /// This consumes the [`SyncRequest`] and returns the updated one.


😃 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

evanlinjin added the api A breaking API change label Apr 25, 2024

evanlinjin added this to the 1.0.0-alpha milestone Apr 25, 2024

evanlinjin self-assigned this Apr 25, 2024

evanlinjin force-pushed the issue/1153 branch 3 times, most recently from b336db8 to 08edd58 Compare April 25, 2024 03:31

evanlinjin and others added 2 commits April 26, 2024 12:55

feat(chain): add sync/full-scan structures for spk-based syncing

cdfec5f

These structures allows spk-based chain-sources to have a universal API. Co-authored-by: Steve Myers <steve@notmandatory.org>

evanlinjin force-pushed the issue/1153 branch from 08edd58 to 546538b Compare April 26, 2024 04:56

feat(esplora)!: update to use new sync/full-scan structures

0f94f24

evanlinjin force-pushed the issue/1153 branch from 546538b to 0f94f24 Compare April 26, 2024 07:10

storopoli reviewed Apr 26, 2024

View reviewed changes

feat(chain): SyncRequest now uses ExactSizeIterators

c0374a0

This allows the caller to track sync progress.

evanlinjin requested review from LLFourn and notmandatory April 27, 2024 12:54

notmandatory mentioned this pull request Apr 28, 2024

Add wallet sync_request and full_scan_request functions #1194

Closed

8 tasks

notmandatory approved these changes Apr 29, 2024

View reviewed changes

evanlinjin merged commit 08fac47 into bitcoindevkit:master May 1, 2024
12 checks passed

This was referenced May 2, 2024

Release 1.0.0-alpha.10 #1418

Closed

fix: Cargo clippy lints #1421

Merged

evanlinjin mentioned this pull request May 4, 2024

Update bdk_electrum crate to use sync/full-scan structs #1403

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce universal sync/full-scan structures for spk-based syncing #1413

Introduce universal sync/full-scan structures for spk-based syncing #1413

evanlinjin commented Apr 25, 2024

storopoli Apr 26, 2024

evanlinjin Apr 27, 2024

storopoli left a comment

notmandatory left a comment

notmandatory Apr 29, 2024

notmandatory Apr 29, 2024

Introduce universal sync/full-scan structures for spk-based syncing #1413

Introduce universal sync/full-scan structures for spk-based syncing #1413

Conversation

evanlinjin commented Apr 25, 2024

Description

Notes to the reviewers

Changelog notice

Checklists

All Submissions:

New Features:

storopoli Apr 26, 2024

Choose a reason for hiding this comment

evanlinjin Apr 27, 2024

Choose a reason for hiding this comment

storopoli left a comment

Choose a reason for hiding this comment

notmandatory left a comment

Choose a reason for hiding this comment

notmandatory Apr 29, 2024

Choose a reason for hiding this comment

notmandatory Apr 29, 2024

Choose a reason for hiding this comment