Use BDK events in `update_payment_store` #658

Camillarhi · 2025-10-09T16:20:13Z

This PR updates update_payment_store to use BDK 2.2’s WalletEvent stream during sync instead of iterating over the full list of wallet transactions every time. The new event-based approach reduces redundant work and ensures the payment store stays in sync with only the changes that actually occurred.

It also sets up the foundation for RBF support in #628 with WalletEvent::TxReplaced. Since #628 depends on this event handling, this PR should be merged first.

This PR will also address #452

ldk-reviews-bot · 2025-10-09T16:20:16Z

👋 Thanks for assigning @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

ldk-reviews-bot · 2025-10-18T00:01:09Z

🔔 1st Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2025-10-20T00:01:43Z

🔔 2nd Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2025-10-22T00:02:22Z

🔔 3rd Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2025-10-25T00:00:15Z

🔔 4th Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2025-10-27T00:00:59Z

🔔 5th Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2025-10-29T00:01:52Z

🔔 6th Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

tnull

Excuse the delay here!

Unfortunately I don't think we can make the move until we get corresponding functionality for all chain sources, i.e., also for bitcoind/apply_block. Will raise that with the BDK folks to make some progress.

I now opened bitcoindevkit/bdk_wallet#336 to add the missing APIs we need. In the meantime we can see to get this as close to being mergeable as possible.

tnull · 2025-10-29T11:31:24Z

src/wallet/mod.rs

 				})?;

-				self.update_payment_store(&mut *locked_wallet).map_err(|e| {
+				let events_vec: Vec<WalletEvent> = events.into_iter().collect();


I don't think this re-allocation is necessary?

Thanks! I have removed the re-allocation

tnull · 2025-10-29T11:35:00Z

src/wallet/mod.rs

@@ -723,7 +943,7 @@ impl Listen for Wallet {

 		match locked_wallet.apply_block(block, height) {


Ugh, seems there is no corresponding apply_block_events method. I think we need that before actually moving forward here. Will raise it with the BDK folks.

I saw that the PR on BDK wallet has been merged and added to the next release milestone. This will be updated as soon as there is a new release on BDK wallet

tnull · 2025-10-29T11:37:20Z

src/wallet/mod.rs

+		}
+
+		self.payment_store
+			.list_filter(|p| {


Hmm, ugh, that's already slow right now, but will be prohibitively slow when we don't keep our entire payment store in-memory. I think we can't get around adding another persisted lookup table that tracks RBF-Txid to original-Txid.

Yes, this is true, if we have another table, the lookup will be faster and the original Txid can be updated when the RBF-Txid for example, has a confirmed event from BDK

@tnull, I created a new persisted ReplacedTransactionStore that maintains lookups from any txid in an RBF chain to its associated payment. Store entries are automatically cleaned up when any transaction in the chain confirms. This keeps lookups fast even with large payment histories.

@tnull, I created a new persisted ReplacedTransactionStore that maintains lookups from any txid in an RBF chain to its associated payment. Store entries are automatically cleaned up when any transaction in the chain confirms. This keeps lookups fast even with large payment histories.

Huh, why do we need a whole other module/store for this? Let's just use a HashMap<Txid, Txid> and be done with it? Or do we need all that additionally tracked data somehow?

I guess we could use a DataStore implementation for this, but I don't quite see why we need to track ConfirmationStatus and latest_update_timestamp?

Huh, why do we need a whole other module/store for this? Let's just use a HashMap<Txid, Txid> and be done with it? Or do we need all that additionally tracked data somehow?

Yes, we need to track this data for faster lookup of replaced transaction IDs instead of a full iteration

I guess we could use a DataStore implementation for this, but I don't quite see why we need to track ConfirmationStatus and latest_update_timestamp?

You are right about the ConfirmationStatus and latest_update_timestamp, they were part of my original design, and as I proceeded with my implementation, I decided to clean the store instead upon confirmation of any of the transactions. I will go ahead and remove them. I also used a DataStore implementation for this.

ldk-reviews-bot · 2025-11-08T02:35:20Z

🔔 1st Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2025-11-10T02:35:49Z

🔔 2nd Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2025-11-12T02:36:22Z

🔔 3rd Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2025-11-15T00:00:24Z

🔔 4th Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2025-11-17T00:01:25Z

🔔 5th Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2025-11-19T00:02:36Z

🔔 6th Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-01-12T00:00:31Z

🔔 1st Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

tnull · 2026-01-13T11:36:37Z

Thanks for the suggestion. You're right. I initially thought list_filter was only checking for the filtered records, but it actually iterates through the entire payment store.
I'll create a PendingPaymentStore that tracks pending payments and replaced txids together. When we get an WalletEvent::TxUnconfirmed event, the payment will be created in the pending store first. Once it reaches ANTI_REORG_DELAY confirmations, it will be created in the main PaymentStore and removed from the pending store. This keeps both stores small and iteration fast.

Cool! Note that we might still want to keep the pending payments in the main store though, as list_payments should continue to return the PENDING entries. Otherwise you'd need to construct the entries on-the-fly from both stores, which I imagine could be more complicated than tracking the same entries twice. But let me know if you find that it works out otherwise.

Please ping me when you switched to the new approach and this is ready for another round of review.

ldk-reviews-bot · 2026-01-15T14:51:05Z

🔔 1st Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-01-17T14:51:50Z

🔔 2nd Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-01-19T14:52:51Z

🔔 3rd Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-01-21T14:53:33Z

🔔 4th Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-01-24T00:00:32Z

🔔 5th Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-01-26T00:01:45Z

🔔 6th Reminder

Hey @tnull! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

tnull

Excuse the delay here once more. I was about to just land this and address my comments myself in a follow-up, however, it seems this needs a rebase by now. Would also be great to at least avoid the churn of first adding the replaced_txids field to then remove it in a later commit.

src/builder.rs

src/payment/store.rs

src/payment/pending_payment_store.rs

Track pending payments with their replaced/conflicting transaction IDs in a separate store. Pending payments are created here on WalletEvent::TxUnconfirmed, then removed once they reach ANTI_REORG_DELAY confirmations. This avoids scanning the entire payment store and enables efficient cleanup.

…sactions Replace the full transaction list scan in `update_payment_store` with handling of BDK's `WalletEvent` stream during sync. This leverages the new events in BDK 2.2, reduces redundant work, and prepares the foundation for reliable RBF/CPFP tracking via `WalletEvent::TxReplaced`

tnull

Thanks for the rebase, I'm going ahead landing this. I opened #770 to track some follow-ups I'd like to address before the next 0.8 release (for simplicity will likely just pick them up myself).

tnull mentioned this pull request Oct 10, 2025

Fix Invalid Pending Payments and Strengthen RBF Tests #647

Closed

Camillarhi force-pushed the payment-store-events-sync branch 9 times, most recently from 75ff700 to d3f7855 Compare October 15, 2025 20:32

Camillarhi marked this pull request as ready for review October 15, 2025 20:35

ldk-reviews-bot requested a review from tnull October 15, 2025 20:36

tnull reviewed Oct 29, 2025

View reviewed changes

Camillarhi force-pushed the payment-store-events-sync branch from d3f7855 to a2c8a55 Compare October 29, 2025 16:08

Camillarhi requested a review from tnull November 6, 2025 02:34

Camillarhi force-pushed the payment-store-events-sync branch 2 times, most recently from ecaae51 to 8609d97 Compare November 6, 2025 10:58

tnull removed their request for review January 13, 2026 11:36

Camillarhi force-pushed the payment-store-events-sync branch 5 times, most recently from 862c5a7 to a677335 Compare January 13, 2026 14:42

Camillarhi requested a review from tnull January 13, 2026 14:50

tnull reviewed Jan 26, 2026

View reviewed changes

src/builder.rs Outdated Show resolved Hide resolved

src/payment/store.rs Outdated Show resolved Hide resolved

src/payment/pending_payment_store.rs Outdated Show resolved Hide resolved

Camillarhi added 2 commits January 26, 2026 13:18

refactor: Extract payment creation logic into create_payment_from_tx

f9834f3

Bump BDK_Wallet to 2.3.0

9c081ce

Camillarhi force-pushed the payment-store-events-sync branch 2 times, most recently from 00b7edc to 76000df Compare January 26, 2026 12:45

Add contains_key method to DataStore

08cbd8b

Camillarhi force-pushed the payment-store-events-sync branch from 76000df to 70288e6 Compare January 26, 2026 13:19

Camillarhi added 2 commits January 26, 2026 14:49

Camillarhi force-pushed the payment-store-events-sync branch from 70288e6 to 9f02ec0 Compare January 26, 2026 13:59

Camillarhi requested a review from tnull January 26, 2026 14:03

tnull mentioned this pull request Jan 27, 2026

Follow-ups to PR #658 #770

Open

6 tasks

tnull approved these changes Jan 27, 2026

View reviewed changes

tnull merged commit ba16c92 into lightningdevkit:main Jan 27, 2026
17 of 18 checks passed

tnull mentioned this pull request Jan 27, 2026

LDK Node v0.8 Tracking issue #698

Open

10 tasks

		@@ -723,7 +943,7 @@ impl Listen for Wallet {

		match locked_wallet.apply_block(block, height) {

Use BDK events in update_payment_store #658

Use BDK events in update_payment_store #658

Uh oh!

Conversation

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

Uh oh!

Uh oh!

ldk-reviews-bot commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ldk-reviews-bot commented Oct 18, 2025

Uh oh!

ldk-reviews-bot commented Oct 20, 2025

Uh oh!

ldk-reviews-bot commented Oct 22, 2025

Uh oh!

ldk-reviews-bot commented Oct 25, 2025

Uh oh!

ldk-reviews-bot commented Oct 27, 2025

Uh oh!

ldk-reviews-bot commented Oct 29, 2025

Uh oh!

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

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tnull Nov 6, 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!

Choose a reason for hiding this comment

Uh oh!

ldk-reviews-bot commented Nov 8, 2025

Uh oh!

ldk-reviews-bot commented Nov 10, 2025

Uh oh!

ldk-reviews-bot commented Nov 12, 2025

Uh oh!

ldk-reviews-bot commented Nov 15, 2025

Uh oh!

ldk-reviews-bot commented Nov 17, 2025

Uh oh!

ldk-reviews-bot commented Nov 19, 2025

Uh oh!

ldk-reviews-bot commented Jan 12, 2026

Uh oh!

tnull commented Jan 13, 2026

Uh oh!

ldk-reviews-bot commented Jan 15, 2026

Uh oh!

ldk-reviews-bot commented Jan 17, 2026

Uh oh!

ldk-reviews-bot commented Jan 19, 2026

Uh oh!

ldk-reviews-bot commented Jan 21, 2026

Uh oh!

ldk-reviews-bot commented Jan 24, 2026

Uh oh!

ldk-reviews-bot commented Jan 26, 2026

Uh oh!

tnull left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Use BDK events in `update_payment_store` #658

Use BDK events in `update_payment_store` #658

Camillarhi commented Oct 9, 2025 •

edited

Loading

ldk-reviews-bot commented Oct 9, 2025 •

edited

Loading

tnull left a comment •

edited

Loading

tnull Nov 6, 2025 •

edited

Loading