Support client_trusts_lsp=true on ldk-node by martinsaposnic · Pull Request #687 · lightningdevkit/ldk-node

martinsaposnic · 2025-10-29T19:05:51Z

implement changes introduced on lightningdevkit/rust-lightning#3838 for rust-lightning.

as discussed, client_trusts_lsp is a flag set at startup.

ldk-reviews-bot · 2025-10-29T19:05:53Z

👋 Thanks for assigning @joostjager 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.

toneloc · 2025-10-29T21:00:11Z

With LDK Node including this PR, I got a compilation error and had to add these imports

use crate::lightning::util::errors::APIError;
use crate::bitcoin::Transaction;

It compiled after that, then I tried to do LSPS2 JIT onboarding, but it failed ("route not found" from the client side).

Note sure if that's an issue with this PR or me.

martinsaposnic · 2025-10-29T21:05:37Z

I’m sorry. This should have been a draft. I just cherry picked the commits from the old PR. I will make CI pass in the next few days

tnull · 2025-10-30T12:02:04Z

Alright, @martinsaposnic please ping me for review as soon as this is ready!

toneloc · 2025-11-11T23:08:28Z

Any progress here? Thanks @martinsaposnic

martinsaposnic · 2025-11-13T14:22:49Z

@tnull this should be ready now. let me know if you have comments

tnull

Actual code changes seem more or less trivial, but this PR needs some more cleanup.

src/event.rs

src/liquidity.rs

tests/integration_tests_rust.rs

tnull · 2025-11-14T10:25:47Z

tests/integration_tests_rust.rs

+	println!("Waiting for funding transaction to be broadcast... It will be there because LSP trusts the client, even though the client has not claimed it yet.");
+	let mut funding_tx_found = false;
+	for _ in 0..500 {
+		std::thread::sleep(std::time::Duration::from_millis(100));


Ugh, let's avoid too many sleeps in tests please, as over time they will considerably slow down our CI for no reason. Please use wait_for_tx for stuff like this, which at least does some polling with back-off.

tests/integration_tests_rust.rs

tnull · 2025-11-17T13:09:53Z

As discussed offline, I'm taking over here to ensure this lands in time for the upcoming 0.7-rc.1. Now added some fixups.

joostjager

Just reviewed the fix up commits, as the preceding commits had already been reviewed.

src/event.rs

joostjager · 2025-11-17T14:18:34Z

src/liquidity.rs

-				Ok(false)
-			}
+	) -> bool {
+		self.lsps2_service.as_ref().map_or(false, |lsps2_service| {


Same q about error logging.

See above, I think continuing and just logging on the manual-broadcast step is preferable.

tests/integration_tests_rust.rs

joostjager · 2025-11-17T14:23:24Z

tests/integration_tests_rust.rs

-			break;
-		}
+	for _ in 0..30 {
+		tokio::time::sleep(std::time::Duration::from_millis(100)).await;


In the happy flow, we are always waiting 3s I think. Is a loop necessary then? Maybe just delay 3s and check.

joostjager · 2025-11-17T14:24:02Z

tests/integration_tests_rust.rs

+		Some(0)
+	);
+
+	// No claim the JIT payment, which should release the funding transaction


Fixed, plus another typo

tnull · 2025-11-17T14:45:42Z

Let me know if I can squash.

tnull · 2025-11-17T14:53:56Z

Now attached another commit that downgrades two explicit panics in handle_event to debug_asserts. While we still shouldn't ever hit an APIMisuseError, the logic there just got a bit more risky, and we really want to avoid crashing the node if ther was, for example, a race condition that has us call into LSPS2ServiceHandler for a counterparty that was just pruned or similar.

joostjager · 2025-11-17T14:58:42Z

Now attached another commit that downgrades two explicit panics in handle_event to debug_asserts. While we still shouldn't ever hit an APIMisuseError, the logic there just got a bit more risky, and we really want to avoid crashing the node if ther was, for example, a race condition that has us call into LSPS2ServiceHandler for a counterparty that was just pruned or similar.

This does sound that we aren't able to fully distinguish between edge cases and actual misuse?

I can see that this might be enough for now though.

Implement changes introduced on lightningdevkit/rust-lightning#3838 as discussed, client_trusts_lsp is a flag set at startup.

Previously, we'd explicitly `panic` on an APIMisuseError. While this error type should still never happen, we avoid explicit panics in favor of `debug_assert`s here.

toneloc · 2025-11-18T20:01:21Z

Awesome guys! Thanks. Could I test this from main? Any config / upgrade considerations to run? Or just the "client_trusts_lsp" flag to "true"?

toneloc · 2025-11-18T20:45:47Z

Awesome guys! Thanks. Could I test this from main? Any config / upgrade considerations to run? Or just the "client_trusts_lsp" flag to "true"?

I'm curious as to how this onboarding flow should work. Would love some docs helping users understand the flow (happy path, unhappy path), and ready help in that regard.

toneloc · 2025-11-19T01:10:05Z

Oh, I think I get it. The test in this PR helped out.

The server side just needs to add the client_trusts_lsp: true flag.

The client needs to get the JIT invoice from a new function receive_via_jit_channel_for_hash.

The client can plug in a custom hash to that function using the below code (I presume this is the safe/canonical way to do it):

  let manual_preimage = PaymentPreimage([42u8; 32]);
  let manual_payment_hash: PaymentHash = manual_preimage.into();

The client should keep track of that preimage (somehow) then upon the PaymentClaimable event the client should call node.bolt11_payment().claim_for_hash() which will signal to the server that the stored channel open transaction should be broadcast (this happens automatically).

tnull · 2025-11-19T08:11:35Z

Oh, I think I get it. The test in this PR helped out.

The server side just needs to add the client_trusts_lsp: true flag.

The client needs to get the JIT invoice from a new function receive_via_jit_channel_for_hash.

The client can plug in a custom hash to that function using the below code (I presume this is the safe/canonical way to do it):
  let manual_preimage = PaymentPreimage([42u8; 32]);
  let manual_payment_hash: PaymentHash = manual_preimage.into();
The client should keep track of that preimage (somehow) then upon the PaymentClaimable event the client should call node.bolt11_payment().claim_for_hash() which will signal to the server that the stored channel open transaction should be broadcast (this happens automatically).

No, this isn't correct. You simple have to set client_trusts_lsp: true on the service side, which might already have been the default, but now we're actually delaying broadcasting the funding transaction until the client claims. Nothing changes on the client side, the regular receive_via_jit_channel should just keep working as expected.

tnull · 2025-11-19T08:20:51Z

Now opened #707 to expand on the docs a bit.

toneloc · 2025-11-19T18:02:29Z

Thank you. Will continue conversation at #707.

martinsaposnic force-pushed the client-trusts-lsp-2 branch 2 times, most recently from 4927b73 to 27dc7b4 Compare October 29, 2025 19:09

ldk-reviews-bot requested a review from tnull October 29, 2025 19:16

martinsaposnic marked this pull request as draft October 29, 2025 21:05

tnull removed their request for review October 30, 2025 12:01

tnull added this to the 0.7 milestone Nov 13, 2025

tnull mentioned this pull request Nov 13, 2025

LDK Node v0.7 Tracking issue #443

Closed

8 tasks

martinsaposnic force-pushed the client-trusts-lsp-2 branch 5 times, most recently from 7835f72 to 97d9e53 Compare November 13, 2025 14:20

martinsaposnic marked this pull request as ready for review November 13, 2025 14:22

ldk-reviews-bot requested a review from tnull November 13, 2025 14:22

tnull reviewed Nov 14, 2025

View reviewed changes

tnull force-pushed the client-trusts-lsp-2 branch from 97d9e53 to 767c564 Compare November 17, 2025 13:08

joostjager reviewed Nov 17, 2025

View reviewed changes

tnull requested a review from joostjager November 17, 2025 14:45

martinsaposnic and others added 2 commits November 17, 2025 16:03

Support client_trusts_lsp=true on ldk-node

aa25497

Implement changes introduced on lightningdevkit/rust-lightning#3838 as discussed, client_trusts_lsp is a flag set at startup.

Avoid explicit panics in handle_event

039aad4

Previously, we'd explicitly `panic` on an APIMisuseError. While this error type should still never happen, we avoid explicit panics in favor of `debug_assert`s here.

tnull force-pushed the client-trusts-lsp-2 branch from e2db7c7 to 039aad4 Compare November 17, 2025 15:03

joostjager approved these changes Nov 17, 2025

View reviewed changes

tnull merged commit 357ac29 into lightningdevkit:main Nov 17, 2025
11 of 17 checks passed

tnull mentioned this pull request Jan 21, 2026

bLIP-52/LSPS2 service: Fully implement client-trusts-LSP flow #479

Closed

Conversation

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

toneloc commented Oct 29, 2025

Uh oh!

martinsaposnic commented Oct 29, 2025

Uh oh!

tnull commented Oct 30, 2025

Uh oh!

toneloc commented Nov 11, 2025

Uh oh!

martinsaposnic commented Nov 13, 2025

Uh oh!

tnull left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tnull Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tnull commented Nov 17, 2025

Uh oh!

joostjager left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

joostjager Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

tnull Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

joostjager Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

tnull Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

joostjager Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

tnull Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

tnull commented Nov 17, 2025

Uh oh!

tnull commented Nov 17, 2025

Uh oh!

joostjager commented Nov 17, 2025

Uh oh!

Uh oh!

toneloc commented Nov 18, 2025

Uh oh!

toneloc commented Nov 18, 2025

Uh oh!

toneloc commented Nov 19, 2025

Uh oh!

tnull commented Nov 19, 2025

Uh oh!

tnull commented Nov 19, 2025

Uh oh!

toneloc commented Nov 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

martinsaposnic commented Oct 29, 2025 •

edited

Loading

ldk-reviews-bot commented Oct 29, 2025 •

edited

Loading