Add wallet sync_request and full_scan_request functions

[notmandatory]

This is WIP. So far only demonstrating with async esplora, if approach looks good I'll do the rest.

Description

Adding new functions to lite client ext modules to simplify scan and sync operations.

This is another attempt to fix #1153.

Notes to the reviewers

Changelog notice

Checklists

All Submissions:

• [x] I've signed all my commits
• [x] I followed the contribution guidelines
• [x] 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

Bugfixes:

• [ ] This pull request breaks the existing API
• [ ] I've added tests to reproduce the issue which are now passing
• [ ] I'm linking the issue being fixed by this PR

[evanlinjin]

@notmandatory do you mind adding a more detailed explanation as to how this pushes us in a direction to have a higher-level method/methods for syncing a wallet?

[LLFourn]

@notmandatory what about doing it like I suggested here: https://github.com/bitcoindevkit/bdk/issues/1153#issuecomment-1752263555

[notmandatory]

Suggestion from https://github.com/bitcoindevkit/bdk/issues/1153#issuecomment-1752263555

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)?:

@evanlinjin where I'm at right now is trying to figure out what wallet data is needed for scan and sync and how to get what we need from existing wallet functions. I should be able to get back to the suggested solution above but am stuck on a few questions maybe you and @LLFourn can help with.

1. does it make sense to have different wallet.start_sync and wallet.start_scan since those operations need different info?

2. things_I_am_interested_in and update need to be tuples right? I don't see any place to put new structs for these without creating unwanted cross-crate-dependencies. It will be trivial to make a impl From to convert the tuple returned from any of the clients back into a wallet Update.

3. For the things_I_am_interested_in does it make sense to clone and collect what I can? I know the goal is to keep this thread safe, is cloning what I need for syncing the only/best option?

What I've found so far is for a client.scan() we need three things from the wallet:

keychain_spks: BTreeMap<K, impl IntoIterator<IntoIter = impl Iterator<Item = (u32, ScriptBuf)> + Send> + Send,>,
local_chain: &LocalChain,
local_tip: Option<CheckPoint>,

and for a client.sync() we need five things:

spks: Vec<ScriptBuf>,
local_chain: &LocalChain,
local_tip: Option<CheckPoint>,
outpoints: Vec<OutPoint>,
txids: Vec<Txid>,

If you guys have any better ideas for these types I'll try to make them work. Thanks!

[notmandatory]

I added Wallet start_scan and start_sync functions as an example of what I have in mind. Once I have confirmation the types are correct I need to add some tests and probably better docs and then also implement scan and sync for EsploraExt and ElectrumExt and update related docs and examples.

[LLFourn]

Suggestion from #1153 (comment)

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)?:

@evanlinjin where I'm at right now is trying to figure out what wallet data is needed for scan and sync and how to get what we need from existing wallet functions. I should be able to get back to the suggested solution above but am stuck on a few questions maybe you and @LLFourn can help with.

1. does it make sense to have different `wallet.start_sync` and `wallet.start_scan` since those operations need different info?

I hadn't thought of making start_scan as well. But I think we're renaming this to rescan and fixing "scan" arguments to just what we need https://github.com/bitcoindevkit/bdk/issues/1112.

2. `things_I_am_interested_in` and `update` need to be tuples right? I don't see any place to put new structs for these without creating unwanted cross-crate-dependencies. It will be trivial to make a `impl From` to convert the tuple returned from any of the clients back into a wallet `Update`.

I don't think they should be tuples? Just a struct definition containing all spks, txids etc. The definition should be in bdk_chain.

3. For the `things_I_am_interested_in` does it make sense to clone and collect what I can? I know the goal is to keep this thread safe, is cloning what I need for syncing the only/best option?

Yes. Things should be cloned and collected out of wallet. FWIW I think we overuse IntoIterator currently. These can just be Vec.

I made another issue to try and collect requirements here: #1195

[notmandatory]

This will need a little rework after #1178 is merged.

[notmandatory]

Also based on poll on discord I'm renaming function names to full_scan and sync, see: https://discord.com/channels/753336465005608961/753367451319926827/1171551358353158164

[notmandatory]

I've rebased post alpha.4 tag.

[benthecarman]

This would be great, would fix the most annoying part of using bdk for me!

[notmandatory]

A quick update. I've spent the past few days trying to figure out how to change the FullScanRequest to be able to add an inspect function for the SPKs but I can't figure out a way to do it. The main issue I run into if I try to use the same basic pattern as with SyncRequest is that I can't "capture" the inspect_spk function when I'm trying to add it to the iterators in the BTreeMap<K, I> that FullScanRequest holds. I think this isn't causing a problem for SyncRequest because it uses collected vectors. I'm open to any ideas on how to do this, the complexity around closures, generics, and thread safety is exceeding my rust capabilities.

[LLFourn]

@notmandatory to do it on FullScanRequest I think your going to need to store a Box<dyn Iterator<...>> for each keychain. Presumably you want to inspect each iterator with the same function. So something like:

self.spks_by_keychain = self.spks_by_keychain.drain(..).map(|keychain, spk_iter| Box::new(spk_iter.inspect(INSPECT_FN)).collect();

This is kinda like what we do now in the examples.

[notmandatory]

I finally got this to build with an inspect function for the FullScanRequest. My next steps are to add tests, update examples to use inspect functions, and verify this API will work with work being done in bitcoindevkit/bdk-ffi#445.

[notmandatory]

I rebased this PR on #1380. In the process I also figured out a way to use the generic K for the function passed into FullScanRequest::inspect_spks(). To do it I had to wrap the Boxed function in an Arc and use the Fn type instead of FnMut. Before I was making a copy of the inspect function for every inspected item, this new way using a reference to the same function which should be more efficient.

[notmandatory]

closing in favor of #1413