-
Notifications
You must be signed in to change notification settings - Fork 30
docs(wallet): expand docs for apply_evicted_txs
#270
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
docs(wallet): expand docs for apply_evicted_txs
#270
Conversation
Pull Request Test Coverage Report for Build 16274314345Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
0121e0f
to
9284767
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you Mammal for the excellent documentation.
ACK 9284767
I left a suggestion for including an example if it won't become too lengthy.
/// influence the wallet's canonicalization logic. | ||
/// | ||
/// [`transactions`]: Wallet::transactions | ||
/// [`start_sync_with_revealed_spks`]: Wallet::start_sync_with_revealed_spks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be great to also include an example here:
/// [`start_sync_with_revealed_spks`]: Wallet::start_sync_with_revealed_spks | |
/// [`start_sync_with_revealed_spks`]: Wallet::start_sync_with_revealed_spks | |
/// ## Example | |
/// | |
/// ```rust | |
/// use bdk_wallet::{Wallet, bitcoin::Txid}; | |
/// use std::collections::HashSet; | |
/// | |
/// let unconfirmed_txids: HashSet<Txid> = wallet | |
/// .transactions() | |
/// .filter(|tx| tx.chain_position.is_unconfirmed()) | |
/// .map(|tx| tx.tx_node.txid) | |
/// .collect(); | |
/// | |
/// let mut evicted_txs = Vec::new(); | |
/// for txid in unconfirmed_txids { | |
/// let tx_node = wallet.tx_graph().full_txs().find(|tx| tx.txid == txid); | |
/// let wallet_tx = wallet.get_tx(txid); | |
/// let is_evicted = wallet_tx.map_or(true, |tx| { | |
/// !tx.chain_position.is_unconfirmed() && !tx.chain_position.is_confirmed() | |
/// }); | |
/// if is_evicted { | |
/// let last_seen = tx_node.map_or(0, |tx| tx.last_seen.unwrap_or(0)); | |
/// evicted_txs.push((txid, last_seen)); | |
/// } | |
/// } | |
/// | |
/// if !evicted_txs.is_empty() { | |
/// wallet.apply_evicted_txs(evicted_txs)?; | |
/// wallet.persist(&mut db)?; | |
/// } |
wallet/src/wallet/mod.rs
Outdated
/// | ||
/// ## Notes | ||
/// | ||
/// - This function is particularly useful when syncing with a blockchain backend that provides |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not clear to me if or when a user needs to use this function. Don't the bdk_* chain clients already evict tx when they notice it missing from the mempool?
@tvpeter @notmandatory One usage example that I know of is in the example_wallet_rpc. |
Description
Expand the API docs for
Wallet::apply_evicted_txs
.This can help to fix #254 by having a section explaining the logic of mempool evictions.
Changelog notice
Checklists
All Submissions:
New Features:
Bugfixes:
This pull request breaks the existing API