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

 			);
 		}

 		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.

Camillarhi · 2025-12-09T19:15:36Z

Thanks for updating! A few comments.

Generally, a lot of things are now happening at the same time (version bumps, code moves/prefactors, actual logic changes, etc). in just these two commits. It would be preferable if you could restructure the commit history to pull out prefactors and unrelated changes to individual commits, which would make following the actual logic changes a lot easier during review. Thanks!

Thanks, I've completed all the requested changes, including restructuring the commits to separate concerns.

ldk-reviews-bot · 2025-12-11T19:16: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-12-15T00:01:00Z

🔔 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-12-17T00:01:34Z

🔔 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-12-20T00:00:21Z

🔔 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-12-22T00:00:54Z

🔔 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-12-24T00:01:37Z

🔔 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 · 2025-12-27T00:00:34Z

🔔 7th 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-12-29T00:01:19Z

🔔 8th 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-12-31T00:02:14Z

🔔 9th 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-03T00:00:16Z

🔔 10th 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-05T00:01:07Z

🔔 11th 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-07T00:02:11Z

🔔 12th 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

Please excuse the considerable delay here!

Revisiting the WalletEvent::ChainTipChanged logic makes me wonder if rather than just tracking 'replaced transactions' in a separate store, it would make more sense to track pending payments (including fields for replaced/conflicting txids) in a separate store.

For one this would avoid us having to scan all entries in the payment store, and it would also allow us to clean up pending payment entries (including associated replaced txids) after ANTI_REORG_DELAY. And even if looking up replaced Txids wouldn't be O(1) anymore, the number of pending payments at any given point of time would be so small that we could still easily iterate over all entries.

Sorry for raising this only now. - Do you have any thoughts on this?

tnull · 2026-01-09T11:56:48Z

src/wallet/mod.rs

+				WalletEvent::ChainTipChanged { new_tip, .. } => {
+					// Get all payments that are Pending with Confirmed status
+					let pending_payments: Vec<PaymentDetails> =
+						self.payment_store.list_filter(|p| {


Hmm, this is unfortunate as it is basically back to scanning all transactions, though this time scanning even all entries in our payment store which might be worse. This makes me wonder if we should, rather than just track conflicting/replaced transactions, track 'pending payments' (with a field for conflicting/replaced txids) to also make this lookup reasonably fast. This might become especially crucial since we're looking to eventually only keep a cache of the payment store in memory, so scanning all entries might at that point mean a lot of slow, costly IO lookups from the KVStore.

tnull · 2026-01-09T12:03:41Z

src/wallet/mod.rs


-		let kind = crate::payment::PaymentKind::Onchain { txid, status: confirmation_status };
+		let existing_payment = self.payment_store.get(&payment_id);
+		let final_conflicting_txids = if let Some(provided_conflicts) = conflicting_txids {


Hmm, tbh, this gets a bit confusing: why are we providing conflicts, and then also looking up conflicts from an existing payment here? Wouldn't it make sense to just have create_payment_from_tx a list of conflicts and populate that at the callsite?

I've removed the conflicting_txids parameter from create_payment_from_tx and moved the logic to the callsite. In WalletEvent::TxReplaced we already have the conflicts from the event, so we just assign them after creating the payment.

tnull · 2026-01-09T12:06:25Z

src/io/utils.rs

 }

+/// Read previously persisted replaced transaction information from the store.
+pub(crate) fn read_replaced_txs<L: Deref>(


Similar to what we did with the other read_ methods this (or rather the pending payments counterpart) should be async now.

tnull · 2026-01-09T12:09:06Z

src/wallet/mod.rs

+		}
+
+		if events.len() > 1 {
+			events.sort_by_key(|e| match e {


Mind adding the explanation you gave w.r.t. why we sort the event as a comment here in the code?

Thanks for the suggestion, I'll add that explanation as a comment

…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`

Camillarhi · 2026-01-09T22:50:51Z

Please excuse the considerable delay here!

Revisiting the WalletEvent::ChainTipChanged logic makes me wonder if rather than just tracking 'replaced transactions' in a separate store, it would make more sense to track pending payments (including fields for replaced/conflicting txids) in a separate store.

For one this would avoid us having to scan all entries in the payment store, and it would also allow us to clean up pending payment entries (including associated replaced txids) after ANTI_REORG_DELAY. And even if looking up replaced Txids wouldn't be O(1) anymore, the number of pending payments at any given point of time would be so small that we could still easily iterate over all entries.

Sorry for raising this only now. - Do you have any thoughts on this?

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.

Introduce a new lookup `ReplacedTransactionStore` that maps old/replaced transaction IDs to their current replacement transaction IDs, enabling reliable tracking of replaced transactions throughout the replacement chain. Key changes: - Add persisted storage for RBF replacement relationships - Link transactions in replacement trees using payment IDs - Remove entire replacement chains from persistence when any transaction in the tree is confirmed

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

Camillarhi force-pushed the payment-store-events-sync branch 6 times, most recently from b442739 to e13e503 Compare December 9, 2025 19:09

Camillarhi requested a review from tnull December 9, 2025 19:15

tnull reviewed Jan 9, 2026

View reviewed changes

Camillarhi added 4 commits January 9, 2026 22:27

refactor: Extract payment creation logic into create_payment_from_tx

d574c9b

Bump BDK_Wallet to 2.3.0

419f350

Add contains_key method to DataStore

a3dded1

Camillarhi force-pushed the payment-store-events-sync branch from e13e503 to e5e120c Compare January 9, 2026 23:28

Camillarhi requested a review from tnull January 9, 2026 23:29

Use BDK events in update_payment_store #658

Are you sure you want to change the base?

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!

Camillarhi commented Dec 9, 2025

Uh oh!

ldk-reviews-bot commented Dec 11, 2025

Uh oh!

ldk-reviews-bot commented Dec 15, 2025

Uh oh!

ldk-reviews-bot commented Dec 17, 2025

Uh oh!

ldk-reviews-bot commented Dec 20, 2025

Uh oh!

ldk-reviews-bot commented Dec 22, 2025

Uh oh!

ldk-reviews-bot commented Dec 24, 2025

Uh oh!

ldk-reviews-bot commented Dec 27, 2025

Uh oh!

ldk-reviews-bot commented Dec 29, 2025

Uh oh!

ldk-reviews-bot commented Dec 31, 2025

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