invoices: add resolution result to htlcResolution

This commit adds the resolution result obtained
while updating an invoice in the registry to
htlcResolution. The field can be used by calling
functions to determine the outcome of the
update and act appropriately.

contractcourt/htlc_incoming_resolver_test.go

@@ -99,6 +99,7 @@ func TestHtlcIncomingResolverExitSettle(t *testing.T) {
 	ctx := newIncomingResolverTestContext(t)
 	ctx.registry.notifyResolution = invoices.NewSettleResolution(
 		testResPreimage, testResCircuitKey, testAcceptHeight,
+		invoices.ResultReplayToSettled,
 	)
 
 	ctx.resolve()
@@ -129,6 +130,7 @@ func TestHtlcIncomingResolverExitCancel(t *testing.T) {
 	ctx := newIncomingResolverTestContext(t)
 	ctx.registry.notifyResolution = invoices.NewFailureResolution(
 		testResCircuitKey, testAcceptHeight,
+		invoices.ResultInvoiceAlreadyCanceled,
 	)
 
 	ctx.resolve()
@@ -147,6 +149,7 @@ func TestHtlcIncomingResolverExitSettleHodl(t *testing.T) {
 	notifyData := <-ctx.registry.notifyChan
 	notifyData.hodlChan <- *invoices.NewSettleResolution(
 		testResPreimage, testResCircuitKey, testAcceptHeight,
+		invoices.ResultSettled,
 	)
 
 	ctx.waitForResult(true)
@@ -174,7 +177,7 @@ func TestHtlcIncomingResolverExitCancelHodl(t *testing.T) {
 	ctx.resolve()
 	notifyData := <-ctx.registry.notifyChan
 	notifyData.hodlChan <- *invoices.NewFailureResolution(
-		testResCircuitKey, testAcceptHeight,
+		testResCircuitKey, testAcceptHeight, invoices.ResultCanceled,
 	)
 
 	ctx.waitForResult(false)

htlcswitch/link.go

@@ -1195,7 +1195,8 @@ func (l *channelLink) processHtlcResolution(resolution invoices.HtlcResolution,
 
 	circuitKey := resolution.CircuitKey
 
-	// Determine required action for the resolution.
+	// Determine required action for the resolution. If the event's preimage is
+	// non-nil, the htlc must be settled. Otherwise, it should be canceled.
 	if resolution.Preimage != nil {
 		l.log.Debugf("received settle resolution for %v", circuitKey)
 
@@ -1205,10 +1206,12 @@ func (l *channelLink) processHtlcResolution(resolution invoices.HtlcResolution,
 		)
 	}
 
-	l.log.Debugf("received cancel resolution for %v", circuitKey)
+	l.log.Debugf("received cancel resolution for %v with outcome: %v",
+		circuitKey, resolution.Outcome)
 
-	// In case of a cancel, always return
+	// The htlc has failed so we cancel it with FailIncorrectDetails. This
-	// incorrect_or_unknown_payment_details in order to avoid leaking info.
+	// error covers invoice failures and hodl cancels (which return it to avoid
+	// leaking information).
 	failure := lnwire.NewFailIncorrectDetails(
 		htlc.pd.Amount, uint32(resolution.AcceptHeight),
 	)
@@ -2844,10 +2847,10 @@ func (l *channelLink) processExitHop(pd *lnwallet.PaymentDescriptor,
 		obfuscator: obfuscator,
 	}
 
+	// If the event is nil, the invoice is being held, so we save payment
+	// descriptor for future reference.
 	if event == nil {
-		// Save payment descriptor for future reference.
 		l.hodlMap[circuitKey] = htlc
-
 		return nil
 	}
 

invoices/invoiceregistry.go

@@ -48,27 +48,32 @@ type HtlcResolution struct {
 
 	// AcceptHeight is the original height at which the htlc was accepted.
 	AcceptHeight int32
+
+	// Outcome indicates the outcome of the invoice registry update.
+	Outcome ResolutionResult
 }
 
 // NewFailureResolution returns a htlc failure resolution.
 func NewFailureResolution(key channeldb.CircuitKey,
-	acceptHeight int32) *HtlcResolution {
+	acceptHeight int32, outcome ResolutionResult) *HtlcResolution {
 
 	return &HtlcResolution{
 		CircuitKey:   key,
 		AcceptHeight: acceptHeight,
+		Outcome:      outcome,
 	}
 }
 
 // NewSettleResolution returns a htlc resolution which is associated with a
 // settle.
 func NewSettleResolution(preimage lntypes.Preimage, key channeldb.CircuitKey,
-	acceptHeight int32) *HtlcResolution {
+	acceptHeight int32, outcome ResolutionResult) *HtlcResolution {
 
 	return &HtlcResolution{
 		Preimage:     &preimage,
 		CircuitKey:   key,
 		AcceptHeight: acceptHeight,
+		Outcome:      outcome,
 	}
 }
 
@@ -347,7 +352,7 @@ func (i *InvoiceRegistry) invoiceEventLoop() {
 		case <-nextReleaseTick:
 			event := autoReleaseHeap.Pop().(*htlcReleaseEvent)
 			err := i.cancelSingleHtlc(
-				event.hash, event.key,
+				event.hash, event.key, ResultMppTimeout,
 			)
 			if err != nil {
 				log.Errorf("HTLC timer: %v", err)
@@ -596,9 +601,11 @@ func (i *InvoiceRegistry) startHtlcTimer(hash lntypes.Hash,
 	}
 }
 
-// cancelSingleHtlc cancels a single accepted htlc on an invoice.
+// cancelSingleHtlc cancels a single accepted htlc on an invoice. It takes
+// a resolution result which will be used to notify subscribed links and
+// resolvers of the details of the htlc cancellation.
 func (i *InvoiceRegistry) cancelSingleHtlc(hash lntypes.Hash,
-	key channeldb.CircuitKey) error {
+	key channeldb.CircuitKey, result ResolutionResult) error {
 
 	i.Lock()
 	defer i.Unlock()
@@ -674,9 +681,11 @@ func (i *InvoiceRegistry) cancelSingleHtlc(hash lntypes.Hash,
 		return fmt.Errorf("htlc %v not found", key)
 	}
 	if htlc.State == channeldb.HtlcStateCanceled {
-		i.notifyHodlSubscribers(
+		resolution := *NewFailureResolution(
-			*NewFailureResolution(key, int32(htlc.AcceptHeight)),
+			key, int32(htlc.AcceptHeight), result,
 		)
+
+		i.notifyHodlSubscribers(resolution)
 	}
 	return nil
 }
@@ -765,7 +774,9 @@ func (i *InvoiceRegistry) NotifyExitHopHtlc(rHash lntypes.Hash,
 
 	// If it isn't recorded, cancel htlc.
 	if !ok {
-		return NewFailureResolution(circuitKey, currentHeight), nil
+		return NewFailureResolution(
+			circuitKey, currentHeight, result,
+		), nil
 	}
 
 	// Determine accepted height of this htlc. If the htlc reached the
@@ -776,7 +787,9 @@ func (i *InvoiceRegistry) NotifyExitHopHtlc(rHash lntypes.Hash,
 
 	switch invoiceHtlc.State {
 	case channeldb.HtlcStateCanceled:
-		return NewFailureResolution(circuitKey, acceptHeight), nil
+		return NewFailureResolution(
+			circuitKey, acceptHeight, result,
+		), nil
 
 	case channeldb.HtlcStateSettled:
 		// Also settle any previously accepted htlcs. The invoice state
@@ -787,17 +800,24 @@ func (i *InvoiceRegistry) NotifyExitHopHtlc(rHash lntypes.Hash,
 				continue
 			}
 
+			// Notify subscribers that the htlcs should be settled
+			// with our peer. Note that the outcome of the
+			// resolution is set based on the outcome of the single
+			// htlc that we just settled, so may not be accurate
+			// for all htlcs.
 			resolution := *NewSettleResolution(
 				invoice.Terms.PaymentPreimage, key,
-				acceptHeight,
+				acceptHeight, result,
 			)
 
 			i.notifyHodlSubscribers(resolution)
 		}
 
-		return NewSettleResolution(
+		resolution := NewSettleResolution(
-			invoice.Terms.PaymentPreimage, circuitKey, acceptHeight,
+			invoice.Terms.PaymentPreimage, circuitKey,
-		), nil
+			acceptHeight, result,
+		)
+		return resolution, nil
 
 	case channeldb.HtlcStateAccepted:
 		// (Re)start the htlc timer if the invoice is still open. It can
@@ -870,7 +890,7 @@ func (i *InvoiceRegistry) SettleHodlInvoice(preimage lntypes.Preimage) error {
 		}
 
 		resolution := *NewSettleResolution(
-			preimage, key, int32(htlc.AcceptHeight),
+			preimage, key, int32(htlc.AcceptHeight), ResultSettled,
 		)
 
 		i.notifyHodlSubscribers(resolution)
@@ -888,7 +908,8 @@ func (i *InvoiceRegistry) CancelInvoice(payHash lntypes.Hash) error {
 
 // cancelInvoice attempts to cancel the invoice corresponding to the passed
 // payment hash. Accepted invoices will only be canceled if explicitly
-// requested to do so.
+// requested to do so. It notifies subscribing links and resolvers that
+// the associated htlcs were canceled if they change state.
 func (i *InvoiceRegistry) cancelInvoiceImpl(payHash lntypes.Hash,
 	cancelAccepted bool) error {
 
@@ -948,7 +969,9 @@ func (i *InvoiceRegistry) cancelInvoiceImpl(payHash lntypes.Hash,
 		}
 
 		i.notifyHodlSubscribers(
-			*NewFailureResolution(key, int32(htlc.AcceptHeight)),
+			*NewFailureResolution(
+				key, int32(htlc.AcceptHeight), ResultCanceled,
+			),
 		)
 	}
 	i.notifyClients(payHash, invoice, channeldb.ContractCanceled)

invoices/invoiceregistry_test.go

@@ -81,16 +81,23 @@ func TestSettleInvoice(t *testing.T) {
 		t.Fatalf("expected acceptHeight %v, but got %v",
 			testCurrentHeight, resolution.AcceptHeight)
 	}
+	if resolution.Outcome != ResultExpiryTooSoon {
+		t.Fatalf("expected expiry too soon, got: %v",
+			resolution.Outcome)
+	}
 
 	// Settle invoice with a slightly higher amount.
 	amtPaid := lnwire.MilliSatoshi(100500)
-	_, err = ctx.registry.NotifyExitHopHtlc(
+	resolution, err = ctx.registry.NotifyExitHopHtlc(
 		testInvoicePaymentHash, amtPaid, testHtlcExpiry, testCurrentHeight,
 		getCircuitKey(0), hodlChan, testPayload,
 	)
 	if err != nil {
 		t.Fatal(err)
 	}
+	if resolution.Outcome != ResultSettled {
+		t.Fatalf("expected settled, got: %v", resolution.Outcome)
+	}
 
 	// We expect the settled state to be sent to the single invoice
 	// subscriber.
@@ -130,6 +137,10 @@ func TestSettleInvoice(t *testing.T) {
 	if resolution.Preimage == nil {
 		t.Fatal("expected settle resolution")
 	}
+	if resolution.Outcome != ResultReplayToSettled {
+		t.Fatalf("expected replay settled, got: %v",
+			resolution.Outcome)
+	}
 
 	// Try to settle again with a new higher-valued htlc. This payment
 	// should also be accepted, to prevent any change in behaviour for a
@@ -144,6 +155,10 @@ func TestSettleInvoice(t *testing.T) {
 	if resolution.Preimage == nil {
 		t.Fatal("expected settle resolution")
 	}
+	if resolution.Outcome != ResultDuplicateToSettled {
+		t.Fatalf("expected duplicate settled, got: %v",
+			resolution.Outcome)
+	}
 
 	// Try to settle again with a lower amount. This should fail just as it
 	// would have failed if it were the first payment.
@@ -157,6 +172,10 @@ func TestSettleInvoice(t *testing.T) {
 	if resolution.Preimage != nil {
 		t.Fatal("expected cancel resolution")
 	}
+	if resolution.Outcome != ResultAmountTooLow {
+		t.Fatalf("expected amount too low, got: %v",
+			resolution.Outcome)
+	}
 
 	// Check that settled amount is equal to the sum of values of the htlcs
 	// 0 and 1.
@@ -287,6 +306,10 @@ func TestCancelInvoice(t *testing.T) {
 		t.Fatalf("expected acceptHeight %v, but got %v",
 			testCurrentHeight, resolution.AcceptHeight)
 	}
+	if resolution.Outcome != ResultInvoiceAlreadyCanceled {
+		t.Fatalf("expected invoice already canceled, got: %v",
+			resolution.Outcome)
+	}
 }
 
 // TestSettleHoldInvoice tests settling of a hold invoice and related
@@ -402,6 +425,10 @@ func TestSettleHoldInvoice(t *testing.T) {
 	if resolution == nil || resolution.Preimage != nil {
 		t.Fatalf("expected htlc to be canceled")
 	}
+	if resolution.Outcome != ResultExpiryTooSoon {
+		t.Fatalf("expected expiry too soon, got: %v",
+			resolution.Outcome)
+	}
 
 	// We expect the accepted state to be sent to the single invoice
 	// subscriber. For all invoice subscribers, we don't expect an update.
@@ -429,6 +456,10 @@ func TestSettleHoldInvoice(t *testing.T) {
 		t.Fatalf("expected acceptHeight %v, but got %v",
 			testCurrentHeight, resolution.AcceptHeight)
 	}
+	if htlcResolution.Outcome != ResultSettled {
+		t.Fatalf("expected result settled, got: %v",
+			htlcResolution.Outcome)
+	}
 
 	// We expect a settled notification to be sent out for both all and
 	// single invoice subscribers.
@@ -535,6 +566,10 @@ func TestCancelHoldInvoice(t *testing.T) {
 		t.Fatalf("expected acceptHeight %v, but got %v",
 			testCurrentHeight, resolution.AcceptHeight)
 	}
+	if resolution.Outcome != ResultReplayToCanceled {
+		t.Fatalf("expected replay to canceled, got %v",
+			resolution.Outcome)
+	}
 }
 
 // TestUnknownInvoice tests that invoice registry returns an error when the
@@ -559,8 +594,10 @@ func TestUnknownInvoice(t *testing.T) {
 	}
 }
 
-// TestSettleMpp tests settling of an invoice with multiple partial payments.
+// TestMppPayment tests settling of an invoice with multiple partial payments.
-func TestSettleMpp(t *testing.T) {
+// It covers the case where there is a mpp timeout before the whole invoice is
+// paid and the case where the invoice is settled in time.
+func TestMppPayment(t *testing.T) {
 	defer timeout()()
 
 	ctx := newTestContext(t)
@@ -597,6 +634,10 @@ func TestSettleMpp(t *testing.T) {
 	if htlcResolution.Preimage != nil {
 		t.Fatal("expected cancel resolution")
 	}
+	if htlcResolution.Outcome != ResultMppTimeout {
+		t.Fatalf("expected mpp timeout, got: %v",
+			htlcResolution.Outcome)
+	}
 
 	// Send htlc 2.
 	hodlChan2 := make(chan interface{}, 1)
@@ -625,9 +666,13 @@ func TestSettleMpp(t *testing.T) {
 	if resolution == nil {
 		t.Fatal("expected a settle resolution")
 	}
+	if resolution.Outcome != ResultSettled {
+		t.Fatalf("expected result settled, got: %v",
+			resolution.Outcome)
+	}
 
 	// Check that settled amount is equal to the sum of values of the htlcs
-	// 0 and 1.
+	// 2 and 3.
 	inv, err := ctx.registry.LookupInvoice(testInvoicePaymentHash)
 	if err != nil {
 		t.Fatal(err)

invoices/update.go

@@ -49,6 +49,10 @@ const (
 	// ResultSettled is returned when we settle an invoice.
 	ResultSettled
 
+	// ResultCanceled is returned when we cancel an invoice and its associated
+	// htlcs.
+	ResultCanceled
+
 	// ResultInvoiceNotOpen is returned when a mpp invoice is not open.
 	ResultInvoiceNotOpen
 
@@ -59,6 +63,10 @@ const (
 	// ResultMppInProgress is returned when we are busy receiving a mpp payment.
 	ResultMppInProgress
 
+	// ResultMppTimeout is returned when an invoice paid with multiple partial
+	// payments times out before it is fully paid.
+	ResultMppTimeout
+
 	// ResultAddressMismatch is returned when the payment address for a mpp
 	// invoice does not match.
 	ResultAddressMismatch