Skip to content

lnpeer: deduct jit channel fees from total amount #10348

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.

Sign up for GitHub

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

Merged
ecdsa merged 2 commits into from
Dec 6, 2025
Merged

lnpeer: deduct jit channel fees from total amount #10348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fb566eb
lnpeer: deduct jit channel fees from total amount
f321x Dec 5, 2025
da99815
lnpeer: deduct JIT fees also for trampoline
ecdsa Dec 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions electrum/lnchannel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -765,8 +765,15 @@ class Channel(AbstractChannel):
def __repr__(self):
return "Channel(%s)"%self.get_id_for_log()

def __init__(self, state: 'StoredDict', *, name=None, lnworker=None, initial_feerate=None, opening_fee=None):
self.opening_fee = opening_fee
def __init__(
self,
state: 'StoredDict', *,
name=None,
lnworker=None,
initial_feerate=None,
jit_opening_fee: Optional[int] = None,
):
self.jit_opening_fee = jit_opening_fee
self.name = name
self.channel_id = bfh(state["channel_id"])
self.short_channel_id = ShortChannelID.normalize(state["short_channel_id"])
Expand Down
21 changes: 14 additions & 7 deletions electrum/lnpeer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1316,9 +1316,12 @@ async def on_open_channel(self, payload):
# store the temp id now, so that it is recognized for e.g. 'error' messages
self.temp_id_to_id[temp_chan_id] = None
self._cleanup_temp_channelids()
channel_opening_fee = open_channel_tlvs.get('channel_opening_fee') if open_channel_tlvs else None
channel_opening_fee_tlv = open_channel_tlvs.get('channel_opening_fee', {})
channel_opening_fee = channel_opening_fee_tlv.get('channel_opening_fee')
if channel_opening_fee:
# todo check that the fee is reasonable
assert is_zeroconf
self.logger.info(f"just-in-time opening fee: {channel_opening_fee} msat")
pass

if self.use_anchors():
Expand Down Expand Up @@ -1433,7 +1436,7 @@ async def on_open_channel(self, payload):
chan_dict,
lnworker=self.lnworker,
initial_feerate=feerate,
opening_fee = channel_opening_fee,
jit_opening_fee = channel_opening_fee,
)
chan.storage['init_timestamp'] = int(time.time())
if isinstance(self.transport, LNTransport):
Expand Down Expand Up @@ -2115,8 +2118,8 @@ def _check_accepted_final_htlc(
log_fail_reason(f"'total_msat' missing from onion")
raise exc_incorrect_or_unknown_pd

if chan.opening_fee:
channel_opening_fee = chan.opening_fee['channel_opening_fee'] # type: int
if chan.jit_opening_fee:
channel_opening_fee = chan.jit_opening_fee
total_msat -= channel_opening_fee
amt_to_forward -= channel_opening_fee
else:
Comment on lines +2121 to 2125
Copy link
Copy Markdown
Member

@SomberNight SomberNight Dec 6, 2025

Choose a reason for hiding this comment

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

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)

Copy link
Copy Markdown
Member

@SomberNight SomberNight Dec 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Member

@SomberNight SomberNight Dec 6, 2025

Choose a reason for hiding this comment

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

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?

electrum/electrum/lnworker.py

Line 1345 in 4d3ead3

funding_sat = 2 * (next_amount_msat_htlc // 1000) # try to fully spend htlcs

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.

Copy link
Copy Markdown
Member

@ecdsa ecdsa Dec 6, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Member Author

@f321x f321x Dec 8, 2025

Choose a reason for hiding this comment

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

Yeah mpp is definitely not intended, i added checks in #10351 to fail htlcs if the sender ignores our invoice and sends mpp anyways.

Expand Down Expand Up @@ -2281,7 +2284,7 @@ def _fulfill_htlc_set(self, payment_key: str, preimage: bytes):
self._fulfill_htlc(chan, htlc_id, preimage)
htlc_set.htlcs.remove(mpp_htlc)
# reset just-in-time opening fee of channel
chan.opening_fee = None
chan.jit_opening_fee = None

def _fulfill_htlc(self, chan: Channel, htlc_id: int, preimage: bytes):
assert chan.hm.is_htlc_irrevocably_added_yet(htlc_proposer=REMOTE, htlc_id=htlc_id)
Expand Down Expand Up @@ -3070,6 +3073,10 @@ def _check_unfulfilled_htlc_set(
return OnionFailureCode.MPP_TIMEOUT, None, None

if mpp_set.resolution == RecvMPPResolution.WAITING:
# 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)
Comment on lines +3076 to +3078
Copy link
Copy Markdown
Member

@SomberNight SomberNight Dec 6, 2025

Choose a reason for hiding this comment

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

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?)

Copy link
Copy Markdown
Member Author

@f321x f321x Dec 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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


# check if set is first stage multi-trampoline payment to us
# first stage trampoline payment:
# is a trampoline payment + we_are_final + payment key is derived from outer onion's payment secret
Expand All @@ -3088,14 +3095,14 @@ def _check_unfulfilled_htlc_set(

if trampoline_payment_key and trampoline_payment_key != payment_key:
# first stage of trampoline payment, the first stage must never get set COMPLETE
if amount_msat >= any_trampoline_onion.amt_to_forward:
if amount_msat >= (any_trampoline_onion.amt_to_forward - jit_opening_fees_msat):
# setting the parent key will mark the htlcs to be moved to the parent set
self.logger.debug(f"trampoline part complete. {len(mpp_set.htlcs)=}, "
f"{amount_msat=}. setting parent key: {trampoline_payment_key}")
self.lnworker.received_mpp_htlcs[payment_key] = mpp_set._replace(
parent_set_key=trampoline_payment_key,
)
elif amount_msat >= total_msat:
elif amount_msat >= (total_msat - jit_opening_fees_msat):
# set mpp_set as completed as we have received the full total_msat
mpp_set = self.lnworker.set_mpp_resolution(
payment_key=payment_key,
Expand Down