lnpeer: deduct jit channel fees from total amount#10348
lnpeer: deduct jit channel fees from total amount#10348ecdsa merged 2 commits intospesmilo:masterfrom
Conversation
f321x
force-pushed
the
fix_jit_channel_openings_regtest
branch
from
48ca037 to
b244b68
Compare
Deduct the just in time channel opening fees from the total amount so htlcs don't get timed out if they come from a just in time channel with opening fee. Related: spesmilo#9584
There was a problem hiding this comment.
this probably requires the same change as below?
- if amount_msat >= any_trampoline_onion.amt_to_forward:
+ if amount_msat >= (any_trampoline_onion.amt_to_forward - jit_opening_fees_msat):
There was a problem hiding this comment.
Ehhhh maybeee... I am not so sure. but maybe I am confused... This might also introduce subtle bugs.
I mean, now you are allowing each first stage to be missing up to jit_opening_fees, and then after they are aggregated, allowing the overall second stage to be missing up to jit_opening_fees.
but you transition every first stage as soon as it is missing less than jit_opening_fees - that means if any of the later-arriving HTLCs are smaller than jit_opening_fees, the payment will not succeed. Because you already transitioned the corresponding first stage set.
Imagine the invoice is for 100 sat, jit_opening_fees = 5 sat, and the sender creates 2 trampoline MPP sets of 50 sat each. So any_trampoline_onion.amt_to_forward == 50 sat. Further imagine the final TF splits the payment to forward 10 htlcs of value 5 sat each. So the final recipient is running this code,
- receives 9 htlcs for set1, sees that 9*5 >= 50-5, transitions it to stage2,
- receives 9 htlcs for set2, sees that 9*5 >= 50-5, transitions it to stage2,
- subsequent htlcs are rejected and stage2 can never complete
or am I missing something?
(for ref lightning/bolts@bc7a1a0)
This stuff is quite complicated, and it was still a draft, so I think you should have waited for @f321x's response before merging it.
There was a problem hiding this comment.
sorry, I did not see that. indeed, it is quite complicated
There was a problem hiding this comment.
Assuming the payment consists of a single trampoline part, which it should as we request no mpp in the invoice, this should work. In #10351 i made this more explicit by deducting the jit fees from any_trampoline_onion.amt_to_forward.total_msat instead of any_trampoline_onion.amt_to_forward.
ecdsa
force-pushed
the
fix_jit_channel_openings_regtest
branch
from
b244b68 to
da99815
Compare
| # calculate the sum of just in time channel opening fees | ||
| htlc_channels = [self.lnworker.get_channel_by_short_id(scid) for scid in set(h.scid for h in mpp_set.htlcs)] | ||
| jit_opening_fees_msat = sum((c.jit_opening_fee or 0) for c in htlc_channels) |
There was a problem hiding this comment.
what is the SCID before the channel gets mined? Or is it already mined at this point? I am confused.
(To be clear, chan.jit_opening_fee being set means we are the enduser here, and not the LSP, right?)
There was a problem hiding this comment.
what is the SCID before the channel gets mined? Or is it already mined at this point? I am confused.
The funding transaction will only get broadcast by the channel provider once we release the preimage, so the channel is not yet mined and the SCID is None. I wasn't aware of this, it doesn't seem safe to handle channels by scid if the scid can be None for some. Using and storing the plain channel_id instead of the scid in ReceivedMPPHtlc seems like a less error prone approach.
(To be clear, chan.jit_opening_fee being set means we are the enduser here, and not the LSP, right?)
yes
| if chan.jit_opening_fee: | ||
| channel_opening_fee = chan.jit_opening_fee | ||
| total_msat -= channel_opening_fee | ||
| amt_to_forward -= channel_opening_fee | ||
| else: |
There was a problem hiding this comment.
how exactly are JIT channels supposed to work with MPP? Is jit_opening_fee supposed to be paid only once? Here we deduct it per number of first stages I guess??? (in case of trampoline)
There was a problem hiding this comment.
I think I would prefer disabling MPP for JIT channels. For the first invoice when the user does not have a chan yet, just don't signal MPP, and also validate when receiving the payment on the JIT channel that it uses a single HTLC.
I want simple, easy-to-reason-about behaviour.
There was a problem hiding this comment.
I mean, imagine Dave already has a channel, that can receive 900k sats. Dave creates an invoice for 1000k sats. Dave puts routing hints into his invoice, one for the existing chan and one that might trigger the LSP to JIT-open another channel.
Let's say Alice is trying to pay that invoice and is lucky, and manages to figure out this 900k+100k sat split, sending 900k to the old chan, and 100k separately through the LSP. Do we want the LSP to open a new channel based on this smaller 100k amount?
I guess currently we would have the LSP open a chan for 200k sat, and forward the 100k on that?
Line 1345 in 4d3ead3
but what if Alice sends 900k on existing chan,
plus 20k on "new" chan,
plus 20k on "new" chan,
plus 20k on "new" chan,
plus 20k on "new" chan,
plus 20k on "new" chan,
(no trampoline involved)?
Would the LSP now open 5 new JIT channels, each funded for 40k sats?
Like wtf?
Just disable MPP.
There was a problem hiding this comment.
Note that lnworker.get_bolt11_invoice already creates invoices with MPP flags disabled, if the payment requires a just-in-time channel. However, this does not guarantee that the sender will not attempt a MPP.
There was a problem hiding this comment.
Yeah mpp is definitely not intended, i added checks in #10351 to fail htlcs if the sender ignores our invoice and sends mpp anyways.
There was a problem hiding this comment.
Ehhhh maybeee... I am not so sure. but maybe I am confused... This might also introduce subtle bugs.
I mean, now you are allowing each first stage to be missing up to jit_opening_fees, and then after they are aggregated, allowing the overall second stage to be missing up to jit_opening_fees.
but you transition every first stage as soon as it is missing less than jit_opening_fees - that means if any of the later-arriving HTLCs are smaller than jit_opening_fees, the payment will not succeed. Because you already transitioned the corresponding first stage set.
Imagine the invoice is for 100 sat, jit_opening_fees = 5 sat, and the sender creates 2 trampoline MPP sets of 50 sat each. So any_trampoline_onion.amt_to_forward == 50 sat. Further imagine the final TF splits the payment to forward 10 htlcs of value 5 sat each. So the final recipient is running this code,
- receives 9 htlcs for set1, sees that 9*5 >= 50-5, transitions it to stage2,
- receives 9 htlcs for set2, sees that 9*5 >= 50-5, transitions it to stage2,
- subsequent htlcs are rejected and stage2 can never complete
or am I missing something?
(for ref lightning/bolts@bc7a1a0)
This stuff is quite complicated, and it was still a draft, so I think you should have waited for @f321x's response before merging it.
3 participants
Deduct the just in time channel opening fees from a htlc sets total amount so htlcs don't get timed out if they come from a just in time channel with opening fee.
Fixes the regtest on master, howevertrampoline paymentsmightneed some additional changes.Related: #9584