Return wallet events when applying updates and blocks (3.0 milestone) #319

notmandatory · 2025-09-26T02:10:40Z

Description

Cherry-pick of commits from #310 and #336.

Refactored Wallet apply updates or blocks to return events and removed v2.x *_events functions.

Notes to the reviewers

I decided to remove the *_events functions added in bdk_wallet-2.2 and 2.3 releases and return events from the original apply_* functions since 3.0 is a breaking milestone and I want to remove the redundant functions.

Changelog notice

BREAKING CHANGES

Refactored Wallet apply updates or blocks to return Vec<WalletEvent>
- Wallet::apply_update
- Wallet::apply_block
- Wallet::apply_block_connected_to
Removed redundant functions:
- Wallet::apply_update_events
- Wallet::apply_block_events
- Wallet::apply_block_connected_to_events

Checklists

All Submissions:

I've signed all my commits
I followed the contribution guidelines
I ran just p before pushing

New Features:

I've added tests for the new feature
I've added docs for the new feature
This pull request breaks the existing API

ValuedMammal

Hey @notmandatory thanks for your work on this! I left some comments related to code quality. As always, nits are non-blocking.

wallet/src/wallet/mod.rs

src/wallet/mod.rs

src/wallet/event.rs

ValuedMammal · 2025-09-29T23:39:12Z

src/wallet/event.rs

+    if chain_tip1 != chain_tip2 {
+        events.push(WalletEvent::ChainTipChanged {
+            old_tip: chain_tip1,
+            new_tip: chain_tip2,
+        });
+    }


I think it would be helpful to know which blocks were connected and/or disconnected.

Do you mean a list of connected/disconnected block ids? I didn't add any other info here because @tnull didn't need it for LDK's use case. Do you have some use or user in mind for this extra info?

Yeah I mean the block IDs. It's just a more complete way of describing how the chain tip changed.

@ValuedMammal are you thinking of something like adding connected and disconnected fields to this variant:

/// The latest chain tip known to the wallet changed. ChainTipChanged { /// Previous chain tip. old_tip: BlockId, /// New chain tip. new_tip: BlockId, /// Newly connected blocks. connected: Vec<BlockId>, /// Newly disconnected blocks. disconnected: Vec<BlockId>, },

I'm still not clear on how this info would be used by a wallet user since it's more of an internal detail.

I've seen a similar thing used in LDK, for example Cache and Listen, but don't know if it's also applicable in this context.

wallet/src/wallet/event.rs

wallet/tests/wallet_event.rs

wallet/src/wallet/event.rs

coveralls · 2025-10-05T20:18:59Z

Pull Request Test Coverage Report for Build 18893642494

Details

158 of 161 (98.14%) changed or added relevant lines in 3 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.2%) to 85.16%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/wallet/event.rs	79	82	96.34%

Totals
Change from base Build 18891447990:	0.2%
Covered Lines:	7110
Relevant Lines:	8349

💛 - Coveralls

notmandatory · 2025-10-29T00:48:18Z

Addressed initial comments from @ValuedMammal and rebased on master branch.

Next steps:

remove Wallet::apply_update_events and change Wallet::apply_update to return events instead
investigate returning events from Wallet::apply_block
~~add examples for getting affected UTXOs/addresses from tx related WalletEvents~~

notmandatory · 2025-11-01T00:34:26Z

I will cherry-pick and incorporate commits from #336 for apply_block_events and apply_block_connected_to_events when it's merged.

notmandatory · 2025-11-13T16:42:58Z

Finished cherry-picking from #336 and rebased on master branch.

codecov · 2025-11-13T16:45:05Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.78%. Comparing base (d825f8c) to head (45b3cd0).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #319      +/-   ##
==========================================
+ Coverage   85.24%   85.78%   +0.53%     
==========================================
  Files          23       24       +1     
  Lines        8229     8348     +119     
==========================================
+ Hits         7015     7161     +146     
+ Misses       1214     1187      -27

Flag	Coverage Δ
rust	`85.78% <100.00%> (+0.53%)`	⬆️

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.

# Conflicts: # src/wallet/event.rs

WalletEvent is a enum of user facing events that are generated when a sync update is applied to a wallet using the Wallet::apply_update_events function.

per suggestions from ValuedMammal: 1. re-export WalletEvent type 2. add comments to wallet_events function 3. rename ambiguous variable names in wallet_events from cp to pos 4. remove signing from wallet_event tests 5. change wallet_events function assert_eq to debug_asset_eq 6. update ADR 0003 decision outcome and add option 4 re: creating events only from Update

Previously, we added a new `Wallet::apply_update_events` method that returned `WalletEvent`s. Unfortunately, no corresponding APIs were added for the `apply_block` counterparts. Here we fix this omission.

Co-authored-by: Steve Myers <github@notmandatory.org>

Also did minor cleanup of apply_update_events tests.

BREAKING CHANGE: 1. changed functions to return WalletEvents: - Wallet::apply_update - Wallet::apply_block - Wallet::apply_block_connected_to 2. removed functions: - Wallet::apply_update_events - Wallet::apply_block_events - Wallet::apply_block_connected_to_events

notmandatory · 2025-12-06T00:49:56Z

Refactored to return events from original apply_update, apply_block* functions, removed *_events functions, see 66cedb4.

notmandatory · 2025-12-09T02:51:54Z

Added one more commit with some docs improvements and refactored out helper method based on suggestions from Claude LLM 🤖.

- Enhanced WalletEvent documentation with clearer explanations - Expanded apply_update() docs with examples and persistence warnings - Extracted collect_wallet_txs() to reduce code duplication - Fixed minor typos and improved code clarity 🤖 LLM-assisted changes

notmandatory · 2025-12-09T03:09:07Z

I haven't added an example for getting affected UTXOs/addresses from a Transaction related WalletEvents. This is a bit out of scope for this PR and should get it's own PR, either to add an example or add a helper function to Wallet to do it.

old

notmandatory · 2025-12-10T22:38:08Z

I haven't added an example for getting affected UTXOs/addresses from a Transaction related WalletEvents. This is a bit out of scope for this PR and should get it's own PR, either to add an example or add a helper function to Wallet to do it.

My thinking is to provide a new helper method Wallet::send_and_received_txouts to accomplish this. See: bitcoindevkit/bdk#2081

ValuedMammal · 2025-12-16T06:32:07Z

In 95e681a: f Duplicate event logic rather than business logic

If this is a fixup commit I guess it can be squashed into the previous one.

ValuedMammal · 2025-12-16T06:37:52Z

This is a bit out of scope for this PR and should get it's own PR, either to add an example or add a helper function to Wallet to do it.

On second thought isn't this precisely what the user is asking for in issue #6, suggesting we should have something like WalletEvent::UtxoSent, and WalletEvent::UtxoReceived?

github-project-automation bot added this to BDK Wallet Sep 26, 2025

notmandatory self-assigned this Sep 26, 2025

notmandatory added this to the Wallet 3.0.0 milestone Sep 26, 2025

notmandatory moved this to In Progress in BDK Wallet Sep 26, 2025

notmandatory moved this from In Progress to Todo in BDK Wallet Sep 26, 2025

ValuedMammal previously requested changes Sep 29, 2025

View reviewed changes

ValuedMammal moved this from Todo to In Progress in BDK Wallet Oct 7, 2025

notmandatory force-pushed the feat/changeset_events_300 branch 2 times, most recently from 21eaa1f to 488b373 Compare October 29, 2025 00:11

notmandatory force-pushed the feat/changeset_events_300 branch from 488b373 to 2891853 Compare October 29, 2025 00:54

ValuedMammal mentioned this pull request Oct 30, 2025

Add apply_block_events and apply_block_connected_to_events #336

Merged

5 tasks

thunderbiscuit mentioned this pull request Oct 30, 2025

Lining Up The Big 3.0 PRs #339

Open

notmandatory force-pushed the feat/changeset_events_300 branch 2 times, most recently from 8d580ed to 6c5c755 Compare November 13, 2025 16:42

notmandatory changed the title ~~Return wallet events when applying updates (3.0 milestone)~~ Return wallet events when applying updates and blocks (3.0 milestone) Nov 13, 2025

notmandatory and others added 8 commits December 5, 2025 14:23

docs: fix small typos

cd02d3c

# Conflicts: # src/wallet/event.rs

feat(wallet): add WalletEvent and Wallet::apply_update_events

a100567

WalletEvent is a enum of user facing events that are generated when a sync update is applied to a wallet using the Wallet::apply_update_events function.

docs(wallet): add example to appl_update_events

aa96479

docs: add events ADR 0003

e632fcc

Add apply_block_events and apply_block_connected_to_events

94dc5f4

Previously, we added a new `Wallet::apply_update_events` method that returned `WalletEvent`s. Unfortunately, no corresponding APIs were added for the `apply_block` counterparts. Here we fix this omission.

f Duplicate event logic rather than business logic

95e681a

Co-authored-by: Steve Myers <github@notmandatory.org>

test(wallet): add tests for apply_block_events

e0bef3b

Also did minor cleanup of apply_update_events tests.

notmandatory force-pushed the feat/changeset_events_300 branch from 6c5c755 to e0bef3b Compare December 5, 2025 20:25

notmandatory force-pushed the feat/changeset_events_300 branch from a868aa8 to 66cedb4 Compare December 6, 2025 00:48

notmandatory force-pushed the feat/changeset_events_300 branch from 5f276a6 to 45b3cd0 Compare December 9, 2025 02:59

notmandatory marked this pull request as ready for review December 9, 2025 03:10

luisschwab self-requested a review December 9, 2025 03:12

notmandatory mentioned this pull request Dec 10, 2025

Add sent_and_received_txouts methods to SPK and Keychain TxOut indexes bitcoindevkit/bdk#2081

Open

3 tasks

ValuedMammal approved these changes Dec 16, 2025

View reviewed changes

Return wallet events when applying updates and blocks (3.0 milestone) #319

Are you sure you want to change the base?

Return wallet events when applying updates and blocks (3.0 milestone) #319

Uh oh!

Conversation

notmandatory commented Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Notes to the reviewers

Changelog notice

Checklists

All Submissions:

New Features:

Uh oh!

ValuedMammal left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ValuedMammal Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

notmandatory Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ValuedMammal Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

notmandatory Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

ValuedMammal Dec 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coveralls commented Oct 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 18893642494

Details

💛 - Coveralls

Uh oh!

notmandatory commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

notmandatory commented Nov 1, 2025

Uh oh!

notmandatory commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

notmandatory commented Dec 6, 2025

Uh oh!

notmandatory commented Dec 9, 2025

Uh oh!

notmandatory commented Dec 9, 2025

Uh oh!

notmandatory commented Dec 10, 2025

Uh oh!

ValuedMammal commented Dec 16, 2025

Uh oh!

ValuedMammal commented Dec 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

notmandatory commented Sep 26, 2025 •

edited

Loading

ValuedMammal left a comment •

edited

Loading

notmandatory Oct 1, 2025 •

edited

Loading

coveralls commented Oct 5, 2025 •

edited

Loading

notmandatory commented Oct 29, 2025 •

edited

Loading

notmandatory commented Nov 13, 2025 •

edited

Loading

codecov bot commented Nov 13, 2025 •

edited

Loading