-
Notifications
You must be signed in to change notification settings - Fork 114
Add receive_via_jit_channel_for_hash()
method
#608
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
👋 Hi! I see this is a draft PR. |
591fb3f
to
bd97482
Compare
There was a problem hiding this 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.
src/payment/bolt11.rs
Outdated
/// 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. |
There was a problem hiding this comment.
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.
Sure, I will add tests. Note that I added the |
Yes, and yes please :) |
bd97482
to
f2afdad
Compare
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).
f2afdad
to
bf348fa
Compare
There was a problem hiding this 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 { |
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
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.
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.