Remove HTLC amount restriction

[t-bast]

We previously had a restriction on HTLC amounts to avoid big losses during the early phases of the network, but it shouldn't be necessary anymore.

As long as we honor max_htlc_value_in_flight_msat and implementations provide safe defaults for that parameter, we don't need that additional restriction.

I'm curious to know whether other implementations enforce this restriction or not (eclair doesn't).

[rustyrussell]

Discussed at meeting. c-lightning and rust-lightning do not enforce on recv, LND needs checking.

However, since Electrum does check on recv, this needs a feature bit. I've grabbed the next one in the title.

[rustyrussell]

I added the option, and restored the send-side restriction when it's not set.

[t-bast]

I'm not sure we need a feature bit if that is already the behavior of most of the network today.
If lnd doesn't enforce it either, we won't be able to rely on that feature bit anyway because nodes that don't set it may still send bigger HTLCs, so why add one?

[rustyrussell]

The feature bit is to stop you sending these to electrum, until they upgrade? Otherwise you'll get an error from them on the channel.

[t-bast]

But adding a feature bit now will not stop existing nodes from sending anyway until they upgrade? I guess the decision really depends on lnd's current behavior, let's see what @Roasbeef says. I'm pretty sure they don't enforce it on receive (since we never had reports about this issue before), but the important point is whether they enforce this restriction on send or not.

Fortunately, this doesn't cause a channel close with Electrum (@ghost43 can you confirm my understanding here?). Could we re-use the existing option_support_large_channel instead of adding a new feature bit? I think it would be more consistent.

[SomberNight]

Fortunately, this doesn't cause a channel close with Electrum (@ghost43 can you confirm my understanding here?).

The channel does not get force-closed, the receiver just disconnects the transport.
In practice though the sender seems to retry as soon as the channel is reestablished, so the channel becomes unusable for a while.

[SomberNight]

Looking at lnd code, I think the limits have been removed in lightningnetwork/lnd#3804 and lightningnetwork/lnd#4335.
I guess ~everyone silently hard forked :)

[t-bast]

Thanks for the investigation @ghost43. If that's the case, that means in the currently deployed network, everyone except Electrum accepts receiving these big HTLCs. What do you prefer on the Electrum side:

1. Lifting this limit entirely, unconditionally (and only leverage max_htlc_value_in_flight_msat to configure limits) as proposed in 0c8eba8
2. Lifting this limit conditionally on option_support_large_channel
3. Lifting this limit conditionally on a new feature bit option_wumbo_htlcs as proposed in 8b750d5

My personal preference is for option 1) which has minimal impact on the spec and implementation code (less if conditions!).

[SomberNight]

I would have preferred using option (3) when implementations originally lifted the limit, but now that there are already lots of nodes out there without the restriction, I agree it is easiest to do (1).

It would take time for other implementations to deploy (3) (or (2)) and get it adopted; with (1) it is only Electrum that needs to change. Further, once Electrum releases a new version with the restriction removed, our users who encounter the issue could even upgrade at that point (if they had not upgraded before then).

Still, I am ok with either option.

[t-bast]

I would have preferred using option (3) when implementations originally lifted the limit, but now that there are already lots of nodes out there without the restriction, I agree it is easiest to do (1).

I agree, it's our bad that we didn't notice that earlier and did the right thing then.
As you mention, given the current conditions, I think that 1) is the simplest way forward to get it fixed rather quickly, so 0c8eba8 should be sufficient

[SomberNight]

Btw I am curious, hypothetically, if we went with option (2) or (3) and required some option for wumbo htlcs, what onion error would you suggest a forwarding node to send back?
i.e. imagine a A -> B -> C -> D payment. A and B negotiated the option, but the BC link does not support it. With what onion error should B fail the HTLC?

EDIT: my point is that we might even need a new error just for this...
EDIT2: hmm ok, maybe it could just be a temporary_channel_failure

[t-bast]

Good point, there isn't a great onion error for that...it would be temporary_channel_failure (but it doesn't help the sender understand if he did something wrong) or maybe required_channel_feature_missing (but a bit convoluted).

[rustyrussell]

OK, I have moved the c-lightning implementation to draft (not in this release). Once Electrum is updated and widely deployed, we can remove this sending restriction altogether.

[Roasbeef]

LGTM 💈

[rustyrussell]

Ack. Recommend delaying removing sending for a few weeks(?) until Electrum widely rolled out though.

(t-bast points out, that if this happens to an Electrum node, they can simply upgrade to fix it. But they might lose the channel due to the stuck HTLC first, so still a good idea to wait).

[cdecker]

ACK 6093296

[t-bast]

Merging as discussed during the meeting.