Update `bdk_electrum` crate to use sync/full-scan structs
Fixes #1265 Possibly fixes #1419
Context
Previous changes such as
- Universal structures for full-scan/sync (PR #1413)
- Making
CheckPointlinked list query-able (PR #1369) - Making
Transactions 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):
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 Arcs 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_txto take in anything that satisfiesInto<Arc<Transaction>>. This allows us to reuse theArcpointer of what is being inserted. - Add
tx_cachefield toSyncRequestandFullScanRequest. - Make
Anchortype inFullScanResultgeneric for more flexibility. - Change
ElectrumExtmethods to take inSyncRequest/FullScanRequestand returnSyncResult/FullScanResult. Also update electrum examples accordingly. - Add
ElectrumResultExttrait which allows us to convert the updateTxGraphofSyncResult/FullScanResultforbdk_electrum. - Added an option for
full_scanandsyncto also fetch previousTxOuts to allow for fee calculation.
Checklists
All Submissions:
- [x] I've signed all my commits
- [x] I followed the contribution guidelines
- [x] I ran
cargo fmtandcargo clippybefore committing
New Features:
- [x] I've added tests for the new feature
- [x] I've added docs for the new feature
For the commit message, remember to put a new line between first line and body.
It seems that you intended the sync flow to be:
- Copy the wallet's
TxGraphand pass it to the chain source. - Perform update, and populate
TxGraphwith missing transactions/anchors and return it back to the wallet.
However, the downsides of this approach is the following:
- Because we are cloning the wallet's
TxGraph, andTxGraphstores both transactions and anchors, and only transactions are cheaply copied (not anchors), therfore copying theTxGraphover is non-optimal. - The anchors that are copied from the wallet cannot be used anyway. We need to keep the anchor type in the
TxGrapha generic to support all wallet types. However, we wish to return an updateTxGraphwith theConfirmationTimeHeightAnchorto represent all information available from Electrum. We cannot guarantee that the wallet's anchor type can be transformed intoConfrimastionTimeHeightAnchor, therefore we end up just re-fetching the anchors from Electrum anyway.
Hence, I think it is better to have a transaction cache HashMap<Txid, Arc<Transaction>> and the TxGraph update as a separate concept.
@LagginTimes thanks for cleaning it up!
What do you see as the next logical step to improving the electrum crate, for example in relation to Rearchitect
bdk_electrumsyncing/scanning. #1332 and Option for Electrum and Esplora APIs to include floatingTxOuts in updates for fee calculation #1265
I would tacke #1265 first. This will make bdk_electrum "feature complete". We can worry about the internals of it after 1.0 is released.
In electrum_ext: do we really need the hard-coded genesis txid? Is this condition even possible?
I would assume not, however someone needs to have a look into it. Additionally, the hardcoded txid is not complete as it only includes the genesis coinbase tx for mainnet.
In electrum_ext: Can we add comments for the
populate_with_*functions? what's being populated, who's doing the populating, etc
Good idea.
I will start poking around with the electrum side of #1265 as well.
A test to see whether fee is actually calculated will be even better.
This needs a rebase, merging #1203 broke this.