Add documentation for Electrum's full_scan/sync

[thunderbiscuit]

This PR also serves as an issue; it can't really be merged as is. I just didn't want to open an issue and just ask for better docs and instead decided to open a PR with some unfinished new API docs.

I am working on a page for the Book of BDK that focuses on bdk_wallet + bdk_electrum syncing. A few things have been unclear to me, and I think slight additions to the API docs would fix that for others.

1. I was wondering what exactly this confirmation_time field on the bdk_chain::ConfirmationTimeHeightAnchor type represents. After digging into it (at least for the electrum client), it represents the timestamp specified by the block header where the tx was confirmed. Question: I'd like to confirm that this always the case, e.g. that this is the timestamp used in cases where your client is an Esplora or bitcoind RPC node? If so, my addition to the sentence will make sense and is ready to go. Edit: this is no longer a type after the rebase.
2. I think it'd be great to add context to the fetch_prev_txouts argument on the full_scan and sync methods on the BdkElectrumClient. It says that this is necessary for "fee calculation". What does that mean? I assume it means "for calculating the fee rate on the given transactions"? (let me know if that's wrong). Even so, I'm left wondering what happens if I don't fetch them. Will my fee calculations be just plain wrong? Or will they be unavailable? A bit more context for the caller of the method would be great here. If someone knows the definite answer to this let me know and feel free to propose a doc line and I'll amend the commit!

Checklists

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

[evanlinjin]

1. For all the chain sources right now, the confirmation time is the block header's timestamp. I think this should always be the defacto-definition (so, yes).
2. Fee here is for absolute fee and fee rate (you can't calculate fee rate without absolute fee). It's necessary for "fee calculation" because tx inputs do not contain amounts of prev outputs. So we need to fetch those prev outputs to do (output amounts - input amounts). If you don't include prev outputs, we cannot determine the fee (and associated methods will return None).

[evanlinjin]

Thanks for the initiative on this, better docs are always better.

[LLFourn]

2. Fee here is for absolute fee and fee rate (you can't calculate fee rate without absolute fee). It's necessary for "fee calculation" because tx inputs do not contain amounts of prev outputs. So we need to fetch those prev outputs to do (output amounts - input amounts). If you don't include prev outputs, we cannot determine the fee (and associated methods will return None).

To clarify, this means that txs spending to your wallet from a foreign wallet will not have the input info and so you can't calculate the fee without fetch_prev_txouts. Txs spending from your wallet will have the input info and so you will be able to get the fee regardless of fetch_prev_txouts.

[thunderbiscuit]

Thanks guys! I'll update the PR shortly.

[thunderbiscuit]

This is what I ended up adding:

    /// - `fetch_prev_txouts`: specifies whether we want previous `TxOut`s for fee calculation.
    ///                        Note that this requires additional calls to the Electrum server, but
    ///                        is necessary for calculating the fee on a transaction if your wallet
    ///                        does not own the inputs. Methods like [`Wallet.calculate_fee`] and
    ///                        [`Wallet.calculate_fee_rate`] will return a
    ///                        [`CalculateFeeError::MissingTxOut`] error if those `TxOut`s are not
    ///                        present in the transaction graph.

[thunderbiscuit]

Rebased and ready to go.

[ValuedMammal]

ACK 056f26c

[evanlinjin]

ACK 056f26c

[LagginTimes]

ACK 056f26c

[thunderbiscuit]

Rebased.

[evanlinjin]

re-ACK 7b5657e