Skip to content

Conversation

andrei-21
Copy link
Contributor

This method will allow to implement swap-in functionality with a JIT channel (i.e., swapping an on-chain payment to Lightning by opening a new channel).

Closes #597.

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Aug 12, 2025

👋 I see @joostjager was un-assigned.
If you'd like another reviewer assignment, please click here.

@andrei-21 andrei-21 force-pushed the feature/for-hash branch 2 times, most recently from 591fb3f to bd97482 Compare August 13, 2025 06:04
Copy link
Collaborator

@tnull tnull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for opening this draft. I think given that we explored ~all other options, I'm slowly getting around to believe this is the simplest solution..

This needs some test coverage though. Could you add some coverage asserting that the manual-claiming flow works just as expected in conjunction with JIT channels?

Also, would be great if the commit message could describe the motivation of the change and what the change actually is.

/// channel to us. We'll use its cheapest offer otherwise.
///
/// We will register the given payment hash and emit a [`PaymentClaimable`] event once
/// the inbound payment arrives.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind adding here that we still check that the LSP-withheld fees are what we expect before issuing the event, i.e., users don't have to double-check that before manually claiming.

@andrei-21
Copy link
Contributor Author

Thanks for opening this draft. I think given that we explored ~all other options, I'm slowly getting around to believe this is the simplest solution..

This needs some test coverage though. Could you add some coverage asserting that the manual-claiming flow works just as expected in conjunction with JIT channels?

Also, would be great if the commit message could describe the motivation of the change and what the change actually is.

Sure, I will add tests.

Note that I added the _for_hash method only for receive_via_jit_channel(), not for receive_variable_amount_via_jit_channel() because I do not need it. But would it make sense to add it for symmetry?
Also, should I expose the method in UDL?

@tnull
Copy link
Collaborator

tnull commented Aug 13, 2025

Note that I added the _for_hash method only for receive_via_jit_channel(), not for receive_variable_amount_via_jit_channel() because I do not need it. But would it make sense to add it for symmetry? Also, should I expose the method in UDL?

Yes, and yes please :)

@andrei-21 andrei-21 force-pushed the feature/for-hash branch 2 times, most recently from f2afdad to bf348fa Compare August 14, 2025 07:59
Copy link
Collaborator

@tnull tnull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, basically LGTM, just two nits.

Also, please undraft :)

@@ -360,8 +360,15 @@ impl Bolt11Payment {
}

if let Some(details) = self.payment_store.get(&payment_id) {
let skimmed_fee_msat = match details.kind {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Mind adding a comment here to describe when this would be relevant?

..
} => skimmed_fee_msat,
_ => 0,
};
if let Some(expected_amount_msat) = details.amount_msat {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Let's rename the variable here, as "the expected amount" will be the net amount, i.e., with skimmed_fee_msat already subtracted.

Add corresponding `receive_via_jit_channel_for_hash()` and
`receive_variable_amount_via_jit_channel_for_hash()` methods that
accept a custom payment hash from the user.
These methods allow implementing swap-in functionality with a JIT
channel (i.e., swapping an on-chain payment to Lightning by opening a
new channel).
@andrei-21 andrei-21 marked this pull request as ready for review August 18, 2025 09:10
@tnull tnull requested review from tnull and removed request for joostjager August 18, 2025 10:07
@tnull tnull merged commit 217b398 into lightningdevkit:main Aug 18, 2025
12 of 24 checks passed
@andrei-21 andrei-21 deleted the feature/for-hash branch August 18, 2025 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

receive_via_jit_channel() with custom hash
3 participants