[wallet] Consider having a higher-level method for syncing
Looking at wallet_esplora or wallet_electrum, syncing takes quite a bit of code, and I suppose than 99% of users will just want to update the whole wallet in one big batch, without any fancy optimization. We should consider having a higher level method similar to the old wallet.sync() to reduce the amount of code copy-pasted from our examples.
Maybe we could have sync_esplora, sync_electrum, sync_bitcoind and all of them need a wallet passed in?
I've started preliminary work on this in #1139 :-)
I think there is almost nothing copy pasted between the bitcoind and electrum examples?
In any case they are architecturally different. In the bitcoind example, your application has a thread that is always running and pulling in new blocks and dumping the mempool every so often. In electrum and esplora, you opportunistically sync whenever you think something might have changed i.e. after giving out a new address, or broadcasting a transaction etc
You could certainly do this for electrum and esplora. See: https://github.com/bitcoindevkit/bdk/pull/1139#issuecomment-1744201377 on how I would approach this. Basically Wallet could be capable of producing a bunch of "things I am interested in" + checkpoint and the electrum and esplora code could produce an update from that generically.
So something like:
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)?:
I think there is almost nothing copy pasted between the bitcoind and electrum examples?
Sorry if I wasn't clear, I meant users copying from our examples to their code.
@LLFourn this looks fantastic.
To be clear, I wasn't suggesting to have one method that works for every backend (as wallet.sync() did); I'm just suggesting to hide some of the complexity inside some auxiliary methods (such as wallet.start_sync(), as you said)
Going along with @LLFourn's suggestion, this is my proposal:
Wallet::start_sync returns a struct that implements a trait that gets the start sync data (let's call this StartSyncGetter for now).
pub trait StartSyncGetter {
fn local_chain_tip(&self) -> CheckPoint;
/* all methods required to get initial data to start sync/scan */
}
The chain source has a method that takes in StartSyncGetter and outputs a struct that contains all data/structures needed for the chain source to start fetching data. The returned structure MUST not hold a reference on the StartSyncGetter input.
The chain source has a second method that actually does IO (fetch data).
P.S. I accidentally clicked on "comment". This proposal is premature.
Why do we need a trait for this. What's wrong with a struct that just has all the fields you need.
I don't think this is closed since we are still missing electrum at least right, @evanlinjin?