Add LSPS1 client-side integration #418

tnull · 2024-12-04T14:49:01Z

We add the capability to act as an LSPS1 / bLIP-51 client.

tnull · 2025-01-17T08:47:57Z

Rebased on main after #426 landed.

tnull · 2025-01-30T13:24:52Z

Rebased on main to resolve minor changes.

jkczyz · 2025-01-31T23:01:05Z

src/liquidity.rs

+			Some(Arc::clone(&self.chain_source)),
+			None,
+			None,
+			liquidity_client_config,


Why do both the LiquidityManager manager and each LSPS service need a copy of the config?

Which config are you referring to here? LiquidityClientConfig? We just give that to the LiquidityManager?

Oh, I meant LSPS1ClientConfig and LSPS2ClientConfig2. Each config is cloned from the respective LSPS service, but the originals are never used anywhere else, AFAICT. Just wondering why we hang on to those inside each service. Maybe instead the config should be pulled out of the service and have the builder hold Option<(LSPS1Service, LSPS1ClientConfig)> if the config is not needed later. Then you won't have two copies of each config.

src/liquidity.rs

jkczyz · 2025-01-31T23:35:00Z

src/liquidity.rs

+/// [bLIP-51 / LSPS1]: https://github.com/lightning/blips/blob/master/blip-0051.md
+/// [`Node::lsps1_liquidity`]: crate::Node::lsps1_liquidity
+#[derive(Clone)]
+pub struct LSPS1Liquidity {


Given LSPS2 channels are JIT, I suppose a similar struct won't be needed there? Perhaps the docs should mention how JIT channels are opened.

Yes, this is one open question I had too, would interested in you opinion: should we move the LSPS2-related methods to a similar handler, or leave them on the Bolt11Payment handler? We could even consider exposing them on both I guess?

Yeah, though perhaps a single liquidity handler would be sufficient (i.e., no need for users to care which BLIP the method is defined in). I'm indifferent as to whether we should keep the method in the Bolt11Payment handler. But we should probably cross-link the docs where appropriate.

(i.e., no need for users to care which BLIP the method is defined in)

Unfortunately I think it does matter, as there are many competing standards (e.g., different JIT-channel protocols), so users are already confused why their wrapped-invoice JIT flow doesn't work with our API, for example. So unfortunately I think we currently (still?) need the spec names front-and-center to reduce confusion and make it really clear what we're supporting and what not. :/

But we should probably cross-link the docs where appropriate.

Yeah, that makes sense, will see to add a note.

Right, I figure there at least be one abstraction for the grouping for LSP spec and method naming can differentiate as needed. But if there are competing standards inside that (or the move to BLIPS obsoletes the grouping), then maybe a separate one is needed.

jkczyz · 2025-01-31T23:40:48Z

src/liquidity.rs

+								log_error!(
+									self.logger,
+									"Failed to handle response from liquidity service: {:?}",
+									e
+								);


Will e contain the response?

Hmm, yes, it includes the parsed responses, LSPS1OrderStatus in this case. Given this is Debug and we'd log any bytestreams as such, this should be safe. But probably better to drop logging them here, as the error is not really dependent on the responses?

Yeah, I'm fine with that though maybe include the request_id instead.

.. to align with the rest of the APIs where we usually go `node_id`, `address`, etc.

We add support for LSPS1 liquidity sources. To this end we slightly refactor our logic to first create a `LiquiditySourceBuilder` that then can be used to `build()` the `LiquiditySource` with the configured services.

We add the logic required to send `create_order` requests and check on their status.

We add an `Lsps1Liquidity` API object, mirroring the approach we took with the `payment` APIs.

tnull marked this pull request as draft December 4, 2024 14:49

tnull force-pushed the 2024-07-lsps1-integration branch from a160bf2 to d0eb977 Compare December 4, 2024 14:55

tnull mentioned this pull request Dec 10, 2024

Add LSPS2 service-side support #420

Draft

tnull force-pushed the 2024-07-lsps1-integration branch from d0eb977 to fe59fed Compare December 10, 2024 13:37

tnull force-pushed the 2024-07-lsps1-integration branch 2 times, most recently from 82fc329 to 9d47470 Compare January 10, 2025 12:09

tnull added the blocked on dependent PR label Jan 15, 2025

tnull force-pushed the 2024-07-lsps1-integration branch from 9d47470 to 5e13aa5 Compare January 16, 2025 09:33

tnull marked this pull request as ready for review January 16, 2025 09:33

tnull added this to the 0.5 milestone Jan 16, 2025

tnull force-pushed the 2024-07-lsps1-integration branch from 5e13aa5 to 2780049 Compare January 17, 2025 08:47

tnull removed the blocked on dependent PR label Jan 17, 2025

tnull mentioned this pull request Jan 17, 2025

LDK Node v0.5 Tracking Issue #360

Open

11 tasks

tnull force-pushed the 2024-07-lsps1-integration branch 2 times, most recently from 115f732 to a2593ae Compare January 30, 2025 13:19

tnull force-pushed the 2024-07-lsps1-integration branch 2 times, most recently from 1d68d58 to e3725b4 Compare January 30, 2025 13:41

tnull requested a review from jkczyz January 30, 2025 14:02

jkczyz reviewed Jan 31, 2025

View reviewed changes

tnull added 4 commits February 1, 2025 08:59

Reorder LSPS2 API params

685c5b6

.. to align with the rest of the APIs where we usually go `node_id`, `address`, etc.

Allow to set LSPS1 liquidity source

6a9e3b5

We add support for LSPS1 liquidity sources. To this end we slightly refactor our logic to first create a `LiquiditySourceBuilder` that then can be used to `build()` the `LiquiditySource` with the configured services.

Add create_order logic

ef49d6d

We add the logic required to send `create_order` requests and check on their status.

f Fix indentation

a077da6

tnull force-pushed the 2024-07-lsps1-integration branch 2 times, most recently from ff6b600 to 30fc8bf Compare February 1, 2025 09:04

tnull added 2 commits February 1, 2025 10:06

f More logged fields

5b2aaec

Add LSPS1 API

889d34c

We add an `Lsps1Liquidity` API object, mirroring the approach we took with the `payment` APIs.

tnull force-pushed the 2024-07-lsps1-integration branch from 30fc8bf to 9cec3f4 Compare February 1, 2025 09:06

tnull added 2 commits February 1, 2025 10:09

Add Uniffi bindings for LSPS1 API

53dac83

Update LSPS2 link in payment store

0e590ba

tnull force-pushed the 2024-07-lsps1-integration branch from 9cec3f4 to 0e590ba Compare February 1, 2025 09:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add LSPS1 client-side integration #418

Add LSPS1 client-side integration #418

tnull commented Dec 4, 2024 •

edited

Loading

tnull commented Jan 17, 2025

tnull commented Jan 30, 2025

jkczyz Jan 31, 2025

tnull Feb 1, 2025

jkczyz Feb 3, 2025

jkczyz Jan 31, 2025

tnull Feb 1, 2025

jkczyz Feb 3, 2025

tnull Feb 4, 2025

jkczyz Feb 4, 2025

jkczyz Jan 31, 2025

tnull Feb 1, 2025

jkczyz Feb 3, 2025

Add LSPS1 client-side integration #418

Are you sure you want to change the base?

Add LSPS1 client-side integration #418

Conversation

tnull commented Dec 4, 2024 • edited Loading

tnull commented Jan 17, 2025

tnull commented Jan 30, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tnull commented Dec 4, 2024 •

edited

Loading