lightningnetwork / lnd

Lightning Network Daemon ⚡️
MIT License
7.63k stars 2.07k forks source link

[bug]: contractcourt: max budget used immediately for anchor sweeps #8738

Closed morehouse closed 3 months ago

morehouse commented 4 months ago

For outgoing HTLCs the commitment deadline is calculated incorrectly, causing the entire anchor budget to be spent immediately.

Problem

The commitment deadline is derived from the timelocks of the outgoing HTLCs, instead of the timelocks for the matching upstream (incoming) HTLCs.

https://github.com/lightningnetwork/lnd/blob/c68778d2f39f13617e05b362fb9bbec43e5c7a43/contractcourt/channel_arbitrator.go#L1452-L1480

Since ChannelArbitrator only broadcast commitments for outgoing HTLCs once the timelock has already expired, the resulting deadline for anchor sweeping is always 0 (which is bumped to 1). This causes the sweeper to spend the entire budget immediately.

Solution

The upstream HTLC timelocks need to be used for the deadline instead.

Roasbeef commented 4 months ago

Nice catch! We added FindOutgoingHTLCDeadline to pass into the HTLC timeout resolver so it could use compute the CLTV delta. We can use the same routine here to compute the deltas to determine what the overall commitment deadline should be.

yyforyongyu commented 4 months ago

Thank you for catching this! I forgot to mention I wanna more opinions here re how we decide the deadline in this case. I don't think we can use the CLTV delta directly, as it may cause no blocks left when sweeping the outgoing HTLCs. Essentially, when sweeping the CPFP anchor and outgoing HTLC, the CLTV is the shared deadline. The expected result is, the HTLC is swept within CLTV delta blocks, which means the anchor CPFP must be confirmed at some block within the CLTV delta too.

More aggressive on the CPFP part.
|-CPFP-|-----HTLC-----|

Share the deadlines evenly.
|---CPFP---|---HTLC---|

More aggressive on the HTLC part.
|-----CPFP-----|-HTLC-|

I think as a starting point, we can ask the CPFP anchor and the HTLC to share the deadline evenly, wdyt?

In the long run, I think the deadline used for CPFP anchor sweeping is a function over all the HTLCs' deadlines and values, future stuff tho.

morehouse commented 4 months ago

Thank you for catching this! I forgot to mention I wanna more opinions here re how we decide the deadline in this case. I don't think we can use the CLTV delta directly, as it may cause no blocks left when sweeping the outgoing HTLCs.

This is also something we need to consider for incoming HTLCs, which have a much shorter default window (10 blocks) to confirm the commitment and HTLC-preimage transactions.

Splitting the window in half seems best to me, especially for this shorter 10 block window. Splitting unevenly (e.g., 80-20) would result in too short of deadlines for either the commitment or HTLC-preimage.