[bug]: reverse the order inside invoice settlement flow

[yyforyongyu]

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.

lnd/htlcswitch/link.go

Lines 3285 to 3307 in d321182

	event, err := l.cfg.Registry.NotifyExitHopHtlc(
	invoiceHash, pd.Amount, pd.Timeout, int32(heightNow),
	circuitKey, l.hodlQueue.ChanIn(), payload,
	)
	if err != nil {
	return err
	}
	// Create a hodlHtlc struct and decide either resolved now or later.
	htlc := hodlHtlc{
	pd: pd,
	obfuscator: obfuscator,
	}
	// If the event is nil, the invoice is being held, so we save payment
	// descriptor for future reference.
	if event == nil {
	l.hodlMap[circuitKey] = htlc
	return nil
	}
	// Process the received resolution.
	return l.processHtlcResolution(event, htlc)

• the invoice is marked as settled in NotifyExitHopHtlc, which calls notifyExitHopHtlcLocked.
• the HTLC is settled in return l.processHtlcResolution(event, htlc).

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,

lnd/itest/lnd_funding_test.go

Lines 558 to 569 in d321182

	// Before Dave closes the channel, he needs to check the invoice is
	// settled to avoid an error saying cannot close channel due to active
	// HTLCs.
	ht.AssertInvoiceSettled(dave, resp.PaymentAddr)
	// TODO(yy): remove the sleep once the following bug is fixed.
	// When the invoice is reported settled, the commitment dance is not
	// yet finished, which can cause an error when closing the channel,
	// saying there's active HTLCs. We need to investigate this issue and
	// reverse the order to, first finish the commitment dance, then report
	// the invoice as settled.
	time.Sleep(2 * time.Second)

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]

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