Skip to content

Add apply_unconfirmed_txs_events and apply_evicted_txs_events#388

Open
notmandatory wants to merge 1 commit intobitcoindevkit:masterfrom
notmandatory:feat/mempool_events
Open

Add apply_unconfirmed_txs_events and apply_evicted_txs_events#388
notmandatory wants to merge 1 commit intobitcoindevkit:masterfrom
notmandatory:feat/mempool_events

Conversation

@notmandatory
Copy link
Member

@notmandatory notmandatory commented Feb 26, 2026

Description

fixes #374

Notes to the reviewers

To reduce duplicate code I also refactored all the _events functions to use new wallet_txs_to_map helper.

Changelog notice

  • Add Wallet::apply_unconfirmed_txs_events
  • Add Wallet::apply_evicted_txs_events

Checklists

All Submissions:

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature

@notmandatory notmandatory self-assigned this Feb 26, 2026
@notmandatory notmandatory added the new feature New feature or request label Feb 26, 2026
@notmandatory notmandatory added this to the Wallet 3.0.0 milestone Feb 26, 2026
@notmandatory
Copy link
Member Author

Per discussion in #374 I'm happy to deprecate these _events functions and replace them with some sort of CanonicalView::diff once that feature is available in bdk_chain. But for now I expect we'll be shipping 3.0 with the current bdk_chain 0.23.

If we agree to go this direction for 3.0 then I'll finish adding tests and cherry-pick these commits back to the release/2.x branch.

@codecov
Copy link

codecov bot commented Feb 26, 2026

Codecov Report

❌ Patch coverage is 94.87179% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.73%. Comparing base (c422104) to head (01b1559).

Files with missing lines Patch % Lines
src/wallet/mod.rs 94.87% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #388      +/-   ##
==========================================
+ Coverage   79.17%   79.73%   +0.56%     
==========================================
  Files          24       24              
  Lines        5311     5296      -15     
  Branches      242      242              
==========================================
+ Hits         4205     4223      +18     
+ Misses       1029      996      -33     
  Partials       77       77              
Flag Coverage Δ
rust 79.73% <94.87%> (+0.56%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@notmandatory notmandatory force-pushed the feat/mempool_events branch 2 times, most recently from 4883f54 to c845417 Compare February 26, 2026 18:20
@notmandatory
Copy link
Member Author

Added a couple simple tests. Ready for review.

@notmandatory notmandatory marked this pull request as ready for review February 26, 2026 18:33
@notmandatory notmandatory moved this to Needs Review in BDK Wallet Feb 26, 2026
…events

- refactor _events functions to use new wallet_txs_to_map helper
@notmandatory
Copy link
Member Author

notmandatory commented Feb 27, 2026

Updated with a couple changes suggested by @ValuedMammal (and also by Claude, will get it to review patches before pushing next time 🤖 ).

  • moved new helper function to impl Wallet since it's only used there
  • removed unneeded Result return from new mempool _events functions

Comment on lines +2703 to +2713
fn wallet_txs_to_map<'a>(
txs: impl Iterator<Item = crate::WalletTx<'a>>,
) -> BTreeMap<Txid, (Arc<Transaction>, ChainPosition<ConfirmationBlockTime>)> {
txs.map(|wtx| {
(
wtx.tx_node.txid,
(wtx.tx_node.tx.clone(), wtx.chain_position),
)
})
.collect()
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: It'd be nice if you could call it as self.map_transactions().

Suggested change
fn wallet_txs_to_map<'a>(
txs: impl Iterator<Item = crate::WalletTx<'a>>,
) -> BTreeMap<Txid, (Arc<Transaction>, ChainPosition<ConfirmationBlockTime>)> {
txs.map(|wtx| {
(
wtx.tx_node.txid,
(wtx.tx_node.tx.clone(), wtx.chain_position),
)
})
.collect()
}
fn map_transactions(
&self,
) -> BTreeMap<Txid, (Arc<Transaction>, ChainPosition<ConfirmationBlockTime>)> {
self.transactions()
.map(|canonical_tx| {
(
canonical_tx.tx_node.txid,
(canonical_tx.tx_node.tx, canonical_tx.chain_position),
)
})
.collect()
}

@thunderbiscuit thunderbiscuit mentioned this pull request Feb 27, 2026
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new feature New feature or request

Projects

Status: Needs Review

Development

Successfully merging this pull request may close these issues.

Add APIs to return WalletEvents when applying mempool transactions (or make wallet_events helper pub)

2 participants