(Needs Help): Add documentation for Electrum's full_scan/sync
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.
- I was wondering what exactly this
confirmation_timefield on thebdk_chain::ConfirmationTimeHeightAnchortype 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. - I think it'd be great to add context to the
fetch_prev_txoutsargument on the full_scan and sync methods on theBdkElectrumClient. 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
- [x] I've signed all my commits
- [x] I followed the contribution guidelines
- [x] I ran
cargo fmtandcargo clippybefore committing
- 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).
- 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).
Thanks for the initiative on this, better docs are always better.
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.
Thanks guys! I'll update the PR shortly.
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.
Rebased and ready to go.
ACK 056f26ca611399512964bd69240e98c6f190845b
Rebased.