Introduce `PaymentStore` to store external payment information by andrei-21 · Pull Request #51 · breez/breez-sdk-ldk

andrei-21 · 2026-02-02T10:59:25Z

First, review models.rs, where data structures are defined in a more natural way. The PaymentStore trait abstracts data storage, enabling decoupling of storage logic. The Store in kv_store.rs implements this trait and will later implement SwapStorage as well. This design allows us to bypass persistent storage and instead retrieve payments via NodeApi, enriching them with data from Store.

roeierez

I understand the concept. I am missing the actual use of the data in the new store.

roeierez · 2026-02-10T19:28:41Z

+#[tonic::async_trait]
+pub trait PaymentStore: Send + Sync {
+    async fn set_ln_info(&self, payment_id: &str, info: &LnPaymentInfo) -> SdkResult<()>;
+    async fn set_lnurl_info(&self, payment_id: &str, info: &LnUrlInfo) -> SdkResult<()>;


Missing get_lnurl_info ?

roeierez · 2026-02-10T19:38:47Z

+                };
+                let info = LnUrlInfo::Pay(info);
+                self.payment_store
+                    .set_lnurl_info(&invoice.payment_hash, &info)


I see we still keep thte insert_payment_external_info. Isn't the whole idea that this payment store will replace that?

andrei-21 · 2026-02-11T09:09:22Z

I understand the concept. I am missing the actual use of the data in the new store.

Right, the idea here is to start storing data first. LNURL info is only a fraction of what needs to be stored (also swap info).
This PR is the first step, it stores LNURL data, later I will add storing swap info, eventually all necessary data will be stored in the new store. After that I will replace reading payment info from Breez SDK storage with the new storage.

roeierez · 2026-02-11T17:52:26Z

I understand the concept. I am missing the actual use of the data in the new store.

Right, the idea here is to start storing data first. LNURL info is only a fraction of what needs to be stored (also swap info). This PR is the first step, it stores LNURL data, later I will add storing swap info, eventually all necessary data will be stored in the new store. After that I will replace reading payment info from Breez SDK storage with the new storage.

I find this way hard to follow and review. Doesn't it seems better to introduce a full store implementation with usage and remove the old store? The other option is to introduce partial implementation but with actual usage.
With the current way I find the review not effective

andrei-21 · 2026-02-12T13:22:41Z

Ok, I see. I will try to make a small but complete solution.

andrei-21 · 2026-02-25T09:03:30Z

I implemented storing and retrieving lighting payment info and LNURL info.
(Build for iOS notification plugin fails, I will fix it meanwhile.)

roeierez · 2026-02-25T17:47:01Z

    string? lnurl_metadata;
    string? ln_address;
    string? lnurl_withdraw_endpoint;
+	LnUrlInfo? lnurl_info;


Should we remove now the redundant old lnurl fields from the udl?

Yes, I will remove.

roeierez · 2026-02-25T17:48:41Z

+    pub bolt11: String,
+    pub payment_hash: String,
+    pub destination_pubkey: String,
+    pub description: String,


Should this be optional?

roeierez · 2026-02-25T17:58:38Z

+                        description: invoice
+                            .description
+                            .as_ref()
+                            .or(invoice.description_hash.as_ref())


Aren't we mixing here description and description hash by using the same field?

Well, a bolt11 invoice must have a description or a description hash, since description hashes are not really supported in the ecosystem (neither in Breez SDK) I suggest just to treat it as a string (rather then discarding it). What do think?

I think if you decide not to support it then it shouldn't be used at all. This way it is confusing because the it mislead the user to think it is the description and nothing tells it is actually a description hash.

I will drop description hashes then.

JssDWt

Can/should we delete SqliteStorage now?

JssDWt · 2026-02-26T09:17:25Z

+        for payment_id in payment_ids {
+            match KVStoreAsync::read(self.kv_store.as_ref(), BREEZ_NS, LNURL_INFOS_NS, payment_id)
+                .await


This is a database roundtrip per payment. Can we do something more efficient?

Yes. I made the simplest implementation for now. Later we can optimize it if there is a need. What do you think?

I think this is one where the need to optimize will show up as something really bad.

You suggest to do it now?
I would want to get approval for the whole concept first.

andrei-21 · 2026-02-26T10:27:48Z

Can/should we delete SqliteStorage now?

Not yet. breez_seriveces reads payments status and so one from the SqliteStorage.

Introduce PaymentStore to store external payment information

bd057ec

andrei-21 force-pushed the feature/payment-store branch 3 times, most recently from 01f1b9e to bd057ec Compare February 4, 2026 19:43

andrei-21 changed the title ~~Introduce PaymentStore~~ Introduce PaymentStore to store external payment information Feb 4, 2026

andrei-21 marked this pull request as ready for review February 5, 2026 09:42

andrei-21 requested review from JssDWt and roeierez February 5, 2026 09:42

roeierez requested changes Feb 10, 2026

View reviewed changes

andrei-21 marked this pull request as draft February 12, 2026 13:22

andrei-21 force-pushed the feature/payment-store branch 6 times, most recently from 5421967 to c49276e Compare February 24, 2026 19:07

Add LnUrlInfo to Payment

0a3f49d

andrei-21 marked this pull request as ready for review February 25, 2026 09:01

andrei-21 requested a review from roeierez February 25, 2026 09:03

andrei-21 force-pushed the feature/payment-store branch from c49276e to 0a3f49d Compare February 25, 2026 09:08

roeierez reviewed Feb 25, 2026

View reviewed changes

andrei-21 added 2 commits February 26, 2026 00:00

Remove old lnurl_ fields

160672e

Make description optional, drop description hash

79d2afe

JssDWt reviewed Feb 26, 2026

View reviewed changes

andrei-21 force-pushed the feature/payment-store branch from 5d33903 to 160672e Compare February 26, 2026 10:56

andrei-21 marked this pull request as draft March 9, 2026 08:28

Uh oh!

Conversation

andrei-21 commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

roeierez left a comment

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!

andrei-21 commented Feb 11, 2026

Uh oh!

roeierez commented Feb 11, 2026

Uh oh!

andrei-21 commented Feb 12, 2026

Uh oh!

andrei-21 commented Feb 25, 2026

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!

JssDWt left a comment

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!

andrei-21 commented Feb 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

andrei-21 commented Feb 2, 2026 •

edited

Loading