lightningnetwork / lnd

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

[bug]: reverse the order inside invoice settlement flow #7463

Open yyforyongyu opened 1 year ago

yyforyongyu commented 1 year ago

Background

When updating an invoice state, we'd mark it as settled before the HTLC is locked in first, resulting in a possible edge case that although an invoice is marked as settled, the HTLC is timed out.

https://github.com/lightningnetwork/lnd/blob/d3211822207afd28a9340b015938371340ba2ec9/htlcswitch/link.go#L3285-L3307

With the final settle signal introduced here this issue is mitigated with extra data, but this still needs a proper fix.

Steps to reproduce

Remove the following sleep in testExternalFundingChanPoint and run the itest,

https://github.com/lightningnetwork/lnd/blob/d3211822207afd28a9340b015938371340ba2ec9/itest/lnd_funding_test.go#L558-L569

Expected behavior

When the invoice is reported as settled, the commitment dance should be finished.

Possible Solutions

  1. Reverse the steps so the HTLC is locked in before the invoice is marked as settled, or,
  2. Add a new state PendingSettle to the invoice and replace it with the current Settled state. Then we only mark it as settled AFTER the HTLC lock-in.
joostjager commented 1 year ago

An alternative name for a new PendingSettle state could be SettleRequested.