lightningnetwork / lnd

Lightning Network Daemon ⚡️
MIT License
7.73k stars 2.09k forks source link

[bug]: lnd should allow HTLC receiver to dip into its channel reserve #8249

Open t-bast opened 12 months ago

t-bast commented 12 months ago

Since the recent on-chain fees spike, we've noticed that many of our lnd peers increased the commit tx feerate to the point where they are just above their channel reserve, without keeping a buffer for the increased commit tx fees for an incoming HTLC. Those channels are thus stuck, because of the very old issue described in https://github.com/lightning/bolts/pull/740

Since we implemented splicing in eclair, we realized that the receiver of an HTLC should always accept incoming HTLCs that make the receiver temporarily drop below their reserve (because of the increased commit tx fees). It is the sender that is taking a risk by doing so. The spec does not tell receivers to treat this as an error, but lnd (and cln) do. This is very annoying, because we could unblock those channels if lnd accepted incoming HTLCs in that situation. Could you relax that requirement instead of sending an invalid update error?

ziggie1984 commented 12 months ago

I think this will be covered in #8096, where we intorduce the feebuffer, but I am not sure whether we should allow going below the reserve because with the feebuffer its very unlikely to happen anyways. Let's imagine we have 1 outgoing + 1 incoming htlc, both of them are added to the commitment in one cycle and would need to dip the local into its reserve to succeed. Would it be allowed, because if I add the outgoing first and then the incoming, your requirement would hold (always allow the incoming to dip the opener into its reserves) but what if we apply them in the other order, incoming first than outgoing ? Should we fail then ? How do you handle this case, or the case you described at the beginning in eclair ?

t-bast commented 12 months ago

I think this will be covered in https://github.com/lightningnetwork/lnd/pull/8096, where we intorduce the feebuffer

Good, that will ensure future channel won't end up stuck. But that doesn't help unblock existing channels (until the feerate decreases and an update_fee decreases the commitment feerate), and won't be sufficient when implementing splicing (when the non-initiator splices a large amount in).

we should allow going below the reserve because with the feebuffer its very unlikely to happen anyways.

As a receiver you should always allow it, you are 100% benefiting from this if your peer lets you go below your reserve. I don't see any reason to disallow it (that's why the spec does not tell you to disallow it).

Let's imagine we have 1 outgoing + 1 incoming htlc, both of them are added to the commitment in one cycle

I think you're assuming that you perform those checks when sending/receiving commit_sig, but I believe that's incorrect, those checks must be done when sending/receiving update_add_htlc. You still must not send an HTLC that would make you go below your reserve, but you should allow receiving an HTLC that would make you go below your reserve. There can't be any race here because of that asymmetry between sending and receiving.

In eclair, we simply do the following:

t-bast commented 12 months ago

This is happening here: https://github.com/ACINQ/eclair/blob/d4a498cdd63c6bc4463a73159d1b0384effb3a9e/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala#L461

ziggie1984 commented 10 months ago

https://github.com/lightningnetwork/lnd/pull/8096 introduced the feebuffer, but allowing the remote side to dip us below channelreserve and vice versa will be done in the follow-up PR.

t-bast commented 10 months ago

Thanks!