Return wallet events when applying updates #310

notmandatory · 2025-09-13T03:15:16Z

Description

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

fixes #6

Notes to the reviewers

I also modified wallet test_utils to add a new_wallet_and_funding_update function that returns a new wallet with an update that funds it. I used this new function in the original new_funded_wallet so there's no duplication.

Changelog notice

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

coveralls · 2025-09-13T03:21:07Z

Pull Request Test Coverage Report for Build 17985287671

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.096%

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

Totals
Change from base Build 17554145933:	0.2%
Covered Lines:	6783
Relevant Lines:	7971

💛 - Coveralls

notmandatory · 2025-09-17T02:35:41Z

After I get a 👍 from @tnull on the WalletEvent enum variants I'll finish up the PR and move it out of draft. I went with a more granular number of variants than we discussed so that this feature can be used more cleanly in on-chain wallet apps.

I'd also like to get feedback from any other wallet devs and the FFI team on how this could help with building more responsive wallet apps.

A few TODOs I have left are:

add simple examples for using the events
create an ADR for this feature

I'm not planning to add an events version of Wallet::apply_block for 2.2 since it's not required by current users and expands the scope of this PR too much.

After this PR is done I'll create a breaking PRs for the 3.0 milestone that deprecates the new function and modifies the function signature of Wallet::apply_update and Wallet::apply_block instead.

tnull

Generally looks good! I left some comments. Mostly I still think that some of these variants are too verbose and/or could be combined, potentially by tracking some optional fields. But no strong opinion/no blocker from my side.

wallet/src/wallet/event.rs

tnull · 2025-09-17T07:18:22Z

wallet/src/wallet/event.rs

+    /// A confirmed transaction is now confirmed in a new block.
+    ///
+    /// This can happen after a chain reorg.
+    TxConfirmedNewBlock {


Maybe TxConfirmationStatusChanged or similar?

I combined with TxConfirmed and added old_block_time to indicate the Tx changed blocks instead of using a new event.

wallet/src/wallet/event.rs

I looked/experimented with BDK, particularly the `bdk_chain`, and I think [this method](https://docs.rs/bdk_chain/latest/bdk_chain/struct.CheckPoint.html#method.insert) is sufficient to remove the store entirely instead of deprecating it. Since every block header is now emitted, the `bdk_kyoto` integration can make use of this to avoid starting on an orphan block. Reorganizations don't need to be explicitly emitted. Rather, the node can just emit whatever chain it is on and `CheckPoint::insert` will reconsile the changes. It looks like there will also be nice [user facing events](bitcoindevkit/bdk_wallet#310), so explicitly informing about a re-org might even be returned in the wallet update. This Kyoto now acts as a decision-maker for the the chain of most work, but there is now a single source-of-truth for the history of headers.

notmandatory · 2025-09-17T13:58:07Z

@tnull thanks for the suggestions, I've updated the events to combine NewConfirmed and TxConfirmedNewBlock into TxConfirmed. And NewUnconfirmed into TxUnconfirmed. In both cases using a old_block_time: Option<ConfirmationBlockTime> field to indicate if the Tx was previously confirmed or not. Does this make sense?

wallet/src/wallet/event.rs

wallet/tests/wallet_event.rs

notmandatory · 2025-09-18T14:34:50Z

Thanks for the feedback @oleonardolima, fixed.

notmandatory · 2025-09-22T20:45:59Z

I added an example for the Wallet::apply_update_events and added an ADR. Ready to review.

oleonardolima

utACK 0c5f4a4

thunderbiscuit

Tested ACK 0c5f4a4.

I think I'll need to use it and see it in production before I get the feel for whether this is all I want/need and in the format I want it, so it's great that you can release this in the 2.2! We should be able to provide polish to the feature if ever it's needed for the 3.0 release.

Thanks for writing the ADR. Very helpful, now and in the future.

docs/adr/0003_events.md

wallet/src/wallet/event.rs

thunderbiscuit · 2025-09-23T14:31:50Z

wallet/src/wallet/event.rs

+        /// Transaction id.
+        txid: Txid,
+        /// Transaction.
+        tx: Arc<Transaction>,


Wondering if we need the whole transaction here. Feels like the txid is enough information for the "messaging" purpose of this WalletEvent type, but maybe you had something in mind for it?

This is just a pointer to a Transaction that already exists in the Wallet's local chain so doesn't cost much to include it and could be useful for a caller who wants to look through it. But I agree it's not strictly needed since the user has the Txid and could call Wallet::get_tx() to get the full WalletTx which includes the Arc<Transaction> .

I suspect even for the bdk-ffi bindings this field would still be an Arc<Transaction> so shouldn't cost much there to included it either. But I haven't tried to implement the bindings for this yet.

@tnull do you think your project will use this Arc to the full Transaction ?

thunderbiscuit · 2025-09-23T14:32:50Z

wallet/src/wallet/event.rs

+        /// Transaction id.
+        txid: Txid,
+        /// Transaction.
+        tx: Arc<Transaction>,


Same as above. Wondering about the reason for the full tx to be returned here.

wallet/src/wallet/event.rs

notmandatory · 2025-09-24T04:31:11Z

~~I just realized I built this on the master branch so I'll need a followup PR to back port it to the release/2.2 branch. Will do that after this one is merged.~~ EDIT: scratch that, I'm going to rebase this one on release/2.2 and make a new one for master so we can talk about changes on that one for 3.0. All commits are the same but cherry-picked onto the release/2.2 branch.

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.

thunderbiscuit

Re-ACK c6fb9b3.

oleonardolima

ACK c6fb9b3

I looked/experimented with BDK, particularly the `bdk_chain`, and I think [this method](https://docs.rs/bdk_chain/latest/bdk_chain/struct.CheckPoint.html#method.insert) is sufficient to remove the store entirely instead of deprecating it. Since every block header is now emitted, the `bdk_kyoto` integration can make use of this to avoid starting on an orphan block. Reorganizations don't need to be explicitly emitted. Rather, the node can just emit whatever chain it is on and `CheckPoint::insert` will reconsile the changes. It looks like there will also be nice [user facing events](bitcoindevkit/bdk_wallet#310), so explicitly informing about a re-org might even be returned in the wallet update. This Kyoto now acts as a decision-maker for the the chain of most work, but there is now a single source-of-truth for the history of headers.

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

notmandatory changed the title ~~Return wallet events when committing new change sets~~ Return wallet events when applying updates Sep 13, 2025

notmandatory self-assigned this Sep 13, 2025

notmandatory added the new feature New feature or request label Sep 13, 2025

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

notmandatory added this to the Wallet 2.2.0 milestone Sep 13, 2025

notmandatory force-pushed the feat/changeset_events branch from 5e28992 to 1ce9292 Compare September 13, 2025 03:36

notmandatory mentioned this pull request Sep 16, 2025

Dev Call Agenda September 16 2025 bitcoindevkit/bdk-ffi#844

Closed

notmandatory force-pushed the feat/changeset_events branch 5 times, most recently from 764b4be to 0c3361c Compare September 17, 2025 02:24

notmandatory force-pushed the feat/changeset_events branch from 0c3361c to 14f847a Compare September 17, 2025 02:50

tnull reviewed Sep 17, 2025

View reviewed changes

rustaceanrob mentioned this pull request Sep 17, 2025

Remove rusqlite dependency 2140-dev/kyoto#489

Merged

notmandatory force-pushed the feat/changeset_events branch from 14f847a to 9b1af09 Compare September 17, 2025 13:48

notmandatory force-pushed the feat/changeset_events branch from 9b1af09 to 57efed0 Compare September 18, 2025 01:39

oleonardolima reviewed Sep 18, 2025

View reviewed changes

wallet/src/wallet/event.rs Outdated Show resolved Hide resolved

wallet/src/wallet/event.rs Outdated Show resolved Hide resolved

wallet/tests/wallet_event.rs Outdated Show resolved Hide resolved

notmandatory force-pushed the feat/changeset_events branch from 57efed0 to 0c5a087 Compare September 18, 2025 14:33

notmandatory force-pushed the feat/changeset_events branch 2 times, most recently from 72e13bc to 8112805 Compare September 22, 2025 00:32

notmandatory marked this pull request as ready for review September 22, 2025 00:41

notmandatory requested a review from ValuedMammal as a code owner September 22, 2025 00:41

notmandatory force-pushed the feat/changeset_events branch from 8112805 to 0c5f4a4 Compare September 22, 2025 20:22

oleonardolima reviewed Sep 23, 2025

View reviewed changes

thunderbiscuit approved these changes Sep 23, 2025

View reviewed changes

thunderbiscuit mentioned this pull request Sep 24, 2025

Consider adding a rust-toolchain.toml file #314

Closed

notmandatory force-pushed the feat/changeset_events branch 2 times, most recently from 5dfb09e to 0a176ce Compare September 24, 2025 02:23

notmandatory moved this from In Progress to Needs Review in BDK Wallet Sep 24, 2025

notmandatory modified the milestones: Wallet 2.2.0, Wallet 3.0.0 Sep 24, 2025

notmandatory added 4 commits September 24, 2025 12:51

docs: fix small typos

8b2efb3

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

fe88a73

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

e1bcce2

docs: add events ADR 0003

c6fb9b3

notmandatory changed the base branch from master to release/2.2 September 24, 2025 17:52

notmandatory force-pushed the feat/changeset_events branch from 0a176ce to c6fb9b3 Compare September 24, 2025 18:00

thunderbiscuit approved these changes Sep 24, 2025

View reviewed changes

notmandatory modified the milestones: Wallet 3.0.0, Wallet 2.2.0 Sep 24, 2025

oleonardolima approved these changes Sep 24, 2025

View reviewed changes

notmandatory mentioned this pull request Sep 25, 2025

Socratic Seminar #36 Topics NashBitDevs/nashbitdevs.org#107

Closed

notmandatory merged commit d77fd95 into bitcoindevkit:release/2.2 Sep 26, 2025
20 checks passed

github-project-automation bot moved this from Needs Review to Done in BDK Wallet Sep 26, 2025

This was referenced Sep 26, 2025

Add user-facing events for applying updates after syncing #6

Closed

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

Draft

tnull mentioned this pull request Sep 26, 2025

Enhance onchain transaction management lightningdevkit/ldk-node#628

Open

ovitrif mentioned this pull request Sep 30, 2025

Introduce IntentTracker #257

Draft

14 tasks

ovitrif mentioned this pull request Oct 29, 2025

Include onchain transactions in events lightningdevkit/ldk-node#448

Draft

Return wallet events when applying updates #310

Return wallet events when applying updates #310

Uh oh!

Conversation

notmandatory commented Sep 13, 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!

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

Uh oh!

Pull Request Test Coverage Report for Build 17985287671

Details

💛 - Coveralls

Uh oh!

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

Uh oh!

Uh oh!

tnull left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tnull Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

notmandatory Sep 17, 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!

Uh oh!

Uh oh!

notmandatory commented Sep 17, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

notmandatory commented Sep 18, 2025

Uh oh!

notmandatory commented Sep 22, 2025

Uh oh!

oleonardolima left a comment

Choose a reason for hiding this comment

Uh oh!

thunderbiscuit 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!

Uh oh!

Uh oh!

thunderbiscuit Sep 23, 2025

Choose a reason for hiding this comment

Uh oh!

notmandatory Sep 24, 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!

thunderbiscuit Sep 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

thunderbiscuit left a comment

Choose a reason for hiding this comment

Uh oh!

oleonardolima left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

notmandatory commented Sep 13, 2025 •

edited

Loading

coveralls commented Sep 13, 2025 •

edited

Loading

notmandatory commented Sep 17, 2025 •

edited

Loading

notmandatory Sep 17, 2025 •

edited

Loading

thunderbiscuit left a comment •

edited

Loading

notmandatory Sep 24, 2025 •

edited

Loading

notmandatory commented Sep 24, 2025 •

edited

Loading