Relax relaying requirement on expiry

[t-bast]

We previously refused to relay HTLCs that would result in a low expiry delta for the next node.
However it's not our decision to make, it's the remote's.
We should forward these HTLCs and let the remote decide whether they fail them because the expiry is too close or fulfill them.

[pm47]

How will that not cause force-close if we remove the check in sendAdd but not in receiveAdd?

I think this change makes sense, but I'm a bit uncomfortable with accepting HTLCs that are possibly already expired; curious what other implementations are doing?

[t-bast]

How will that not cause force-close if we remove the check in sendAdd but not in receiveAdd?

I'm not seeing any related check in receiveAdd...can you detail?

There's nothing in the spec that relates to that check we were doing, and nothing that says a low HTLC expiry should trigger a force-close. On the contrary, a low HTLC expiry should simply trigger an UpdateFailHtlc where you either include a channel update if you're a relaying node and your cltv_expiry_delta wasn't satisfied, or an error message saying the final parameters were incorrect if you're the recipient.

[t-bast]

I think this change makes sense, but I'm a bit uncomfortable with accepting HTLCs that are possibly already expired; curious what other implementations are doing?

This cannot happen, as these would not pass the check on our cltv_expiry_delta in the relayOrFail in ChannelRelayer

[pm47]

How will that not cause force-close if we remove the check in sendAdd but not in receiveAdd?

I'm not seeing any related check in receiveAdd...can you detail?

Indeed! This inconsistency is by itself a red flag for current implementation. The culprit is #687. Looking at the description of that PR, the rationale for case b) is not very convincing (guilty!). At least I'm consistent with myself from 2 years ago.

I think this change makes sense, but I'm a bit uncomfortable with accepting HTLCs that are possibly already expired; curious what other implementations are doing?

This cannot happen, as these would not pass the check on our cltv_expiry_delta in the relayOrFail in ChannelRelayer

We will accept and sign them, but not relay them. I agree this should be harmless, it just feels strange to sign a htlc for which the counter-party can immediately claim the corresponding output using the timeout branch.

[t-bast]

We will accept and sign them, but not relay them. I agree this should be harmless, it just feels strange to sign a htlc for which the counter-party can immediately claim the corresponding output using the timeout branch.

It's true that it feels a bit weird! But I think the right behavior in that case is to accept the HTLC and immediately fail it (since there's no way to un-add an HTLC), which will be our behavior.

Otherwise if we added a force-close in that case, I believe it could be exploited to force us to close channels.
Imagine we have A -> B -> C -> D -> E and we are D.
A malicious B could force us to close the C -> D channel by simply waiting the right amount of block before forwarding to C.