multi: replace htlcResolution with an interface

This commit repalces the htlcResolution struct with an interface.
This interface is implemeted by failure, settle and accept resolution
structs. Only settles and fails are exported because the existing
code that handles htlc resolutions uses a nil resolution to indicate
that a htlc was accepted. The accept resolution is used internally
to report on the resolution result of the accepted htlc, but a nil
resolution is surfaced. Further refactoring of all the functions
that call NotifyExitHopHtlc to handle a htlc accept case (rather than
having a nil check) is required.

invoices/invoiceregistry.go

@@ -36,48 +36,6 @@ const (
 	DefaultHtlcHoldDuration = 120 * time.Second
 )
 
-// HtlcResolution describes how an htlc should be resolved. If the preimage
-// field is set, the event indicates a settle event. If Preimage is nil, it is
-// a cancel event.
-type HtlcResolution struct {
-	// Preimage is the htlc preimage. Its value is nil in case of a cancel.
-	Preimage *lntypes.Preimage
-
-	// CircuitKey is the key of the htlc for which we have a resolution
-	// decision.
-	CircuitKey channeldb.CircuitKey
-
-	// 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, 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, outcome ResolutionResult) *HtlcResolution {
-
-	return &HtlcResolution{
-		Preimage:     &preimage,
-		CircuitKey:   key,
-		AcceptHeight: acceptHeight,
-		Outcome:      outcome,
-	}
-}
-
 // RegistryConfig contains the configuration parameters for invoice registry.
 type RegistryConfig struct {
 	// FinalCltvRejectDelta defines the number of blocks before the expiry
@@ -683,7 +641,7 @@ func (i *InvoiceRegistry) cancelSingleHtlc(hash lntypes.Hash,
 		return fmt.Errorf("htlc %v not found", key)
 	}
 	if htlc.State == channeldb.HtlcStateCanceled {
-		resolution := *NewFailureResolution(
+		resolution := NewFailResolution(
 			key, int32(htlc.AcceptHeight), result,
 		)
 
@@ -777,7 +735,7 @@ func (i *InvoiceRegistry) processKeySend(ctx invoiceUpdateCtx) error {
 func (i *InvoiceRegistry) NotifyExitHopHtlc(rHash lntypes.Hash,
 	amtPaid lnwire.MilliSatoshi, expiry uint32, currentHeight int32,
 	circuitKey channeldb.CircuitKey, hodlChan chan<- interface{},
-	payload Payload) (*HtlcResolution, error) {
+	payload Payload) (HtlcResolution, error) {
 
 	mpp := payload.MultiPath()
 
@@ -802,7 +760,7 @@ func (i *InvoiceRegistry) NotifyExitHopHtlc(rHash lntypes.Hash,
 		if err != nil {
 			updateCtx.log(fmt.Sprintf("keysend error: %v", err))
 
-			return NewFailureResolution(
+			return NewFailResolution(
 				circuitKey, currentHeight, ResultKeySendError,
 			), nil
 		}
@@ -817,15 +775,10 @@ func (i *InvoiceRegistry) NotifyExitHopHtlc(rHash lntypes.Hash,
 	}
 
 	switch r := resolution.(type) {
-
-	// A direct resolution was received for this htlc.
-	case *HtlcResolution:
-		return r, nil
-
 	// The htlc is held. Start a timer outside the lock if the htlc should
 	// be auto-released, because otherwise a deadlock may happen with the
 	// main event loop.
-	case *acceptResolution:
+	case *htlcAcceptResolution:
 		if r.autoRelease {
 			err := i.startHtlcTimer(rHash, circuitKey, r.acceptTime)
 			if err != nil {
@@ -833,33 +786,31 @@ func (i *InvoiceRegistry) NotifyExitHopHtlc(rHash lntypes.Hash,
 			}
 		}
 
+		// We return a nil resolution because htlc acceptances are
+		// represented as nil resolutions externally.
+		// TODO(carla) update calling code to handle accept resolutions.
 		return nil, nil
 
+	// A direct resolution was received for this htlc.
+	case HtlcResolution:
+		return r, nil
+
+	// Fail if an unknown resolution type was received.
 	default:
 		return nil, errors.New("invalid resolution type")
 	}
 }
 
-// acceptResolution is returned when the htlc should be held.
-type acceptResolution struct {
-	// autoRelease signals that the htlc should be automatically released
-	// after a timeout.
-	autoRelease bool
-
-	// acceptTime is the time at which this htlc was accepted.
-	acceptTime time.Time
-}
-
 // notifyExitHopHtlcLocked is the internal implementation of NotifyExitHopHtlc
 // that should be executed inside the registry lock.
 func (i *InvoiceRegistry) notifyExitHopHtlcLocked(
 	ctx *invoiceUpdateCtx, hodlChan chan<- interface{}) (
-	interface{}, error) {
+	HtlcResolution, error) {
 
 	// We'll attempt to settle an invoice matching this rHash on disk (if
 	// one exists). The callback will update the invoice state and/or htlcs.
 	var (
-		result            ResolutionResult
+		resolution        HtlcResolution
 		updateSubscribers bool
 	)
 	invoice, err := i.cdb.UpdateInvoice(
@@ -876,8 +827,8 @@ func (i *InvoiceRegistry) notifyExitHopHtlcLocked(
 			updateSubscribers = updateDesc != nil &&
 				updateDesc.State != nil
 
-			// Assign result to outer scope variable.
-			result = res
+			// Assign resolution to outer scope variable.
+			resolution = res
 
 			return updateDesc, nil
 		},
@@ -886,7 +837,7 @@ func (i *InvoiceRegistry) notifyExitHopHtlcLocked(
 	case channeldb.ErrInvoiceNotFound:
 		// If the invoice was not found, return a failure resolution
 		// with an invoice not found result.
-		return NewFailureResolution(
+		return NewFailResolution(
 			ctx.circuitKey, ctx.currentHeight,
 			ResultInvoiceNotFound,
 		), nil
@@ -898,38 +849,40 @@ func (i *InvoiceRegistry) notifyExitHopHtlcLocked(
 		return nil, err
 	}
 
-	ctx.log(result.String())
-
 	if updateSubscribers {
 		i.notifyClients(ctx.hash, invoice, invoice.State)
 	}
 
-	// Inspect latest htlc state on the invoice.
-	invoiceHtlc, ok := invoice.Htlcs[ctx.circuitKey]
+	switch res := resolution.(type) {
+	case *HtlcFailResolution:
+		// Inspect latest htlc state on the invoice. If it is found,
+		// we will update the accept height as it was recorded in the
+		// invoice database (which occurs in the case where the htlc
+		// reached the database in a previous call). If the htlc was
+		// not found on the invoice, it was immediately failed so we
+		// send the failure resolution as is, which has the current
+		// height set as the accept height.
+		invoiceHtlc, ok := invoice.Htlcs[ctx.circuitKey]
+		if ok {
+			res.AcceptHeight = int32(invoiceHtlc.AcceptHeight)
+		}
 
-	// If it isn't recorded, cancel htlc.
-	if !ok {
-		return NewFailureResolution(
-			ctx.circuitKey, ctx.currentHeight, result,
-		), nil
-	}
+		ctx.log(fmt.Sprintf("failure resolution result "+
+			"outcome: %v, at accept height: %v",
+			res.Outcome, res.AcceptHeight))
 
-	// Determine accepted height of this htlc. If the htlc reached the
-	// invoice database (possibly in a previous call to the invoice
-	// registry), we'll take the original accepted height as it was recorded
-	// in the database.
-	acceptHeight := int32(invoiceHtlc.AcceptHeight)
+		return res, nil
 
-	switch invoiceHtlc.State {
-	case channeldb.HtlcStateCanceled:
-		return NewFailureResolution(
-			ctx.circuitKey, acceptHeight, result,
-		), nil
+	// If the htlc was settled, we will settle any previously accepted
+	// htlcs and notify our peer to settle them.
+	case *HtlcSettleResolution:
+		ctx.log(fmt.Sprintf("settle resolution result "+
+			"outcome: %v, at accept height: %v",
+			res.Outcome, res.AcceptHeight))
 
-	case channeldb.HtlcStateSettled:
-		// Also settle any previously accepted htlcs. The invoice state
-		// is leading. If an htlc is marked as settled, we should follow
-		// now and settle the htlc with our peer.
+		// Also settle any previously accepted htlcs. If a htlc is
+		// marked as settled, we should follow now and settle the htlc
+		// with our peer.
 		for key, htlc := range invoice.Htlcs {
 			if htlc.State != channeldb.HtlcStateSettled {
 				continue
@@ -940,34 +893,49 @@ func (i *InvoiceRegistry) notifyExitHopHtlcLocked(
 			// 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, result,
+			htlcSettleResolution := NewSettleResolution(
+				res.Preimage, key,
+				int32(htlc.AcceptHeight), res.Outcome,
 			)
 
-			i.notifyHodlSubscribers(resolution)
+			// Notify subscribers that the htlc should be settled
+			// with our peer.
+			i.notifyHodlSubscribers(htlcSettleResolution)
 		}
 
-		resolution := NewSettleResolution(
-			invoice.Terms.PaymentPreimage, ctx.circuitKey,
-			acceptHeight, result,
-		)
 		return resolution, nil
 
-	case channeldb.HtlcStateAccepted:
-		var resolution acceptResolution
+	// If we accepted the htlc, subscribe to the hodl invoice and return
+	// an accept resolution with the htlc's accept time on it.
+	case *htlcAcceptResolution:
+		invoiceHtlc, ok := invoice.Htlcs[ctx.circuitKey]
+		if !ok {
+			return nil, fmt.Errorf("accepted htlc: %v not"+
+				" present on invoice: %x", ctx.circuitKey,
+				ctx.hash[:])
+		}
+
+		// Determine accepted height of this htlc. If the htlc reached
+		// the invoice database (possibly in a previous call to the
+		// invoice registry), we'll take the original accepted height
+		// as it was recorded in the database.
+		acceptHeight := int32(invoiceHtlc.AcceptHeight)
+
+		ctx.log(fmt.Sprintf("accept resolution result "+
+			"outcome: %v, at accept height: %v",
+			res.outcome, acceptHeight))
 
 		// Auto-release the htlc if the invoice is still open. It can
 		// only happen for mpp payments that there are htlcs in state
 		// Accepted while the invoice is Open.
 		if invoice.State == channeldb.ContractOpen {
-			resolution.acceptTime = invoiceHtlc.AcceptTime
-			resolution.autoRelease = true
+			res.acceptTime = invoiceHtlc.AcceptTime
+			res.autoRelease = true
 
 		}
 
 		i.hodlSubscribe(hodlChan, ctx.circuitKey)
-		return &resolution, nil
+		return res, nil
 
 	default:
 		panic("unknown action")
@@ -1022,7 +990,7 @@ func (i *InvoiceRegistry) SettleHodlInvoice(preimage lntypes.Preimage) error {
 			continue
 		}
 
-		resolution := *NewSettleResolution(
+		resolution := NewSettleResolution(
 			preimage, key, int32(htlc.AcceptHeight), ResultSettled,
 		)
 
@@ -1102,7 +1070,7 @@ func (i *InvoiceRegistry) cancelInvoiceImpl(payHash lntypes.Hash,
 		}
 
 		i.notifyHodlSubscribers(
-			*NewFailureResolution(
+			NewFailResolution(
 				key, int32(htlc.AcceptHeight), ResultCanceled,
 			),
 		)
@@ -1374,7 +1342,7 @@ func (i *InvoiceRegistry) SubscribeSingleInvoice(
 // notifyHodlSubscribers sends out the htlc resolution to all current
 // subscribers.
 func (i *InvoiceRegistry) notifyHodlSubscribers(htlcResolution HtlcResolution) {
-	subscribers, ok := i.hodlSubscriptions[htlcResolution.CircuitKey]
+	subscribers, ok := i.hodlSubscriptions[htlcResolution.CircuitKey()]
 	if !ok {
 		return
 	}
@@ -1391,11 +1359,11 @@ func (i *InvoiceRegistry) notifyHodlSubscribers(htlcResolution HtlcResolution) {
 
 		delete(
 			i.hodlReverseSubscriptions[subscriber],
-			htlcResolution.CircuitKey,
+			htlcResolution.CircuitKey(),
 		)
 	}
 
-	delete(i.hodlSubscriptions, htlcResolution.CircuitKey)
+	delete(i.hodlSubscriptions, htlcResolution.CircuitKey())
 }
 
 // hodlSubscribe adds a new invoice subscription.

invoices/invoiceregistry_test.go

@@ -74,29 +74,38 @@ func TestSettleInvoice(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if resolution.Preimage != nil {
-		t.Fatal("expected cancel resolution")
+	failResolution, ok := resolution.(*HtlcFailResolution)
+	if !ok {
+		t.Fatalf("expected fail resolution, got: %T",
+			resolution)
 	}
-	if resolution.AcceptHeight != testCurrentHeight {
+	if failResolution.AcceptHeight != testCurrentHeight {
 		t.Fatalf("expected acceptHeight %v, but got %v",
-			testCurrentHeight, resolution.AcceptHeight)
+			testCurrentHeight, failResolution.AcceptHeight)
 	}
-	if resolution.Outcome != ResultExpiryTooSoon {
+	if failResolution.Outcome != ResultExpiryTooSoon {
 		t.Fatalf("expected expiry too soon, got: %v",
-			resolution.Outcome)
+			failResolution.Outcome)
 	}
 
 	// Settle invoice with a slightly higher amount.
 	amtPaid := lnwire.MilliSatoshi(100500)
 	resolution, err = ctx.registry.NotifyExitHopHtlc(
-		testInvoicePaymentHash, amtPaid, testHtlcExpiry, testCurrentHeight,
-		getCircuitKey(0), hodlChan, testPayload,
+		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)
+	settleResolution, ok := resolution.(*HtlcSettleResolution)
+	if !ok {
+		t.Fatalf("expected settle resolution, got: %T",
+			resolution)
+	}
+	if settleResolution.Outcome != ResultSettled {
+		t.Fatalf("expected settled, got: %v",
+			settleResolution.Outcome)
 	}
 
 	// We expect the settled state to be sent to the single invoice
@@ -134,12 +143,14 @@ func TestSettleInvoice(t *testing.T) {
 	if err != nil {
 		t.Fatalf("unexpected NotifyExitHopHtlc error: %v", err)
 	}
-	if resolution.Preimage == nil {
-		t.Fatal("expected settle resolution")
+	settleResolution, ok = resolution.(*HtlcSettleResolution)
+	if !ok {
+		t.Fatalf("expected settle resolution, got: %T",
+			resolution)
 	}
-	if resolution.Outcome != ResultReplayToSettled {
+	if settleResolution.Outcome != ResultReplayToSettled {
 		t.Fatalf("expected replay settled, got: %v",
-			resolution.Outcome)
+			settleResolution.Outcome)
 	}
 
 	// Try to settle again with a new higher-valued htlc. This payment
@@ -152,12 +163,14 @@ func TestSettleInvoice(t *testing.T) {
 	if err != nil {
 		t.Fatalf("unexpected NotifyExitHopHtlc error: %v", err)
 	}
-	if resolution.Preimage == nil {
-		t.Fatal("expected settle resolution")
+	settleResolution, ok = resolution.(*HtlcSettleResolution)
+	if !ok {
+		t.Fatalf("expected settle resolution, got: %T",
+			resolution)
 	}
-	if resolution.Outcome != ResultDuplicateToSettled {
+	if settleResolution.Outcome != ResultDuplicateToSettled {
 		t.Fatalf("expected duplicate settled, got: %v",
-			resolution.Outcome)
+			settleResolution.Outcome)
 	}
 
 	// Try to settle again with a lower amount. This should fail just as it
@@ -169,12 +182,14 @@ func TestSettleInvoice(t *testing.T) {
 	if err != nil {
 		t.Fatalf("unexpected NotifyExitHopHtlc error: %v", err)
 	}
-	if resolution.Preimage != nil {
-		t.Fatal("expected cancel resolution")
+	failResolution, ok = resolution.(*HtlcFailResolution)
+	if !ok {
+		t.Fatalf("expected fail resolution, got: %T",
+			resolution)
 	}
-	if resolution.Outcome != ResultAmountTooLow {
+	if failResolution.Outcome != ResultAmountTooLow {
 		t.Fatalf("expected amount too low, got: %v",
-			resolution.Outcome)
+			failResolution.Outcome)
 	}
 
 	// Check that settled amount is equal to the sum of values of the htlcs
@@ -298,17 +313,18 @@ func TestCancelInvoice(t *testing.T) {
 	if err != nil {
 		t.Fatal("expected settlement of a canceled invoice to succeed")
 	}
-
-	if resolution.Preimage != nil {
-		t.Fatal("expected cancel htlc resolution")
+	failResolution, ok := resolution.(*HtlcFailResolution)
+	if !ok {
+		t.Fatalf("expected fail resolution, got: %T",
+			resolution)
 	}
-	if resolution.AcceptHeight != testCurrentHeight {
+	if failResolution.AcceptHeight != testCurrentHeight {
 		t.Fatalf("expected acceptHeight %v, but got %v",
-			testCurrentHeight, resolution.AcceptHeight)
+			testCurrentHeight, failResolution.AcceptHeight)
 	}
-	if resolution.Outcome != ResultInvoiceAlreadyCanceled {
-		t.Fatalf("expected invoice already canceled, got: %v",
-			resolution.Outcome)
+	if failResolution.Outcome != ResultInvoiceAlreadyCanceled {
+		t.Fatalf("expected expiry too soon, got: %v",
+			failResolution.Outcome)
 	}
 }
 
@@ -422,12 +438,14 @@ func TestSettleHoldInvoice(t *testing.T) {
 	if err != nil {
 		t.Fatalf("expected settle to succeed but got %v", err)
 	}
-	if resolution == nil || resolution.Preimage != nil {
-		t.Fatalf("expected htlc to be canceled")
+	failResolution, ok := resolution.(*HtlcFailResolution)
+	if !ok {
+		t.Fatalf("expected fail resolution, got: %T",
+			resolution)
 	}
-	if resolution.Outcome != ResultExpiryTooSoon {
+	if failResolution.Outcome != ResultExpiryTooSoon {
 		t.Fatalf("expected expiry too soon, got: %v",
-			resolution.Outcome)
+			failResolution.Outcome)
 	}
 
 	// We expect the accepted state to be sent to the single invoice
@@ -449,16 +467,21 @@ func TestSettleHoldInvoice(t *testing.T) {
 	}
 
 	htlcResolution := (<-hodlChan).(HtlcResolution)
-	if *htlcResolution.Preimage != testInvoicePreimage {
+	settleResolution, ok := htlcResolution.(*HtlcSettleResolution)
+	if !ok {
+		t.Fatalf("expected settle resolution, got: %T",
+			htlcResolution)
+	}
+	if settleResolution.Preimage != testInvoicePreimage {
 		t.Fatal("unexpected preimage in hodl resolution")
 	}
-	if htlcResolution.AcceptHeight != testCurrentHeight {
+	if settleResolution.AcceptHeight != testCurrentHeight {
 		t.Fatalf("expected acceptHeight %v, but got %v",
-			testCurrentHeight, resolution.AcceptHeight)
+			testCurrentHeight, settleResolution.AcceptHeight)
 	}
-	if htlcResolution.Outcome != ResultSettled {
+	if settleResolution.Outcome != ResultSettled {
 		t.Fatalf("expected result settled, got: %v",
-			htlcResolution.Outcome)
+			settleResolution.Outcome)
 	}
 
 	// We expect a settled notification to be sent out for both all and
@@ -545,8 +568,10 @@ func TestCancelHoldInvoice(t *testing.T) {
 	}
 
 	htlcResolution := (<-hodlChan).(HtlcResolution)
-	if htlcResolution.Preimage != nil {
-		t.Fatal("expected cancel htlc resolution")
+	_, ok := htlcResolution.(*HtlcFailResolution)
+	if !ok {
+		t.Fatalf("expected fail resolution, got: %T",
+			htlcResolution)
 	}
 
 	// Offering the same htlc again at a higher height should still result
@@ -559,16 +584,18 @@ func TestCancelHoldInvoice(t *testing.T) {
 	if err != nil {
 		t.Fatalf("expected settle to succeed but got %v", err)
 	}
-	if resolution.Preimage != nil {
-		t.Fatalf("expected htlc to be canceled")
+	failResolution, ok := resolution.(*HtlcFailResolution)
+	if !ok {
+		t.Fatalf("expected fail resolution, got: %T",
+			resolution)
 	}
-	if resolution.AcceptHeight != testCurrentHeight {
+	if failResolution.AcceptHeight != testCurrentHeight {
 		t.Fatalf("expected acceptHeight %v, but got %v",
-			testCurrentHeight, resolution.AcceptHeight)
+			testCurrentHeight, failResolution.AcceptHeight)
 	}
-	if resolution.Outcome != ResultReplayToCanceled {
+	if failResolution.Outcome != ResultReplayToCanceled {
 		t.Fatalf("expected replay to canceled, got %v",
-			resolution.Outcome)
+			failResolution.Outcome)
 	}
 }
 
@@ -585,16 +612,21 @@ func TestUnknownInvoice(t *testing.T) {
 	// succeed.
 	hodlChan := make(chan interface{})
 	amt := lnwire.MilliSatoshi(100000)
-	result, err := ctx.registry.NotifyExitHopHtlc(
+	resolution, err := ctx.registry.NotifyExitHopHtlc(
 		testInvoicePaymentHash, amt, testHtlcExpiry, testCurrentHeight,
 		getCircuitKey(0), hodlChan, testPayload,
 	)
 	if err != nil {
 		t.Fatal("unexpected error")
 	}
-	if result.Outcome != ResultInvoiceNotFound {
+	failResolution, ok := resolution.(*HtlcFailResolution)
+	if !ok {
+		t.Fatalf("expected fail resolution, got: %T",
+			resolution)
+	}
+	if failResolution.Outcome != ResultInvoiceNotFound {
 		t.Fatalf("expected ResultInvoiceNotFound, got: %v",
-			result.Outcome)
+			failResolution.Outcome)
 	}
 }
 
@@ -646,16 +678,17 @@ func testKeySend(t *testing.T, keySendEnabled bool) {
 	if err != nil {
 		t.Fatal(err)
 	}
-
-	// Expect a cancel resolution with the correct outcome.
-	if resolution.Preimage != nil {
-		t.Fatal("expected cancel resolution")
+	failResolution, ok := resolution.(*HtlcFailResolution)
+	if !ok {
+		t.Fatalf("expected fail resolution, got: %T",
+			resolution)
 	}
+
 	switch {
-	case !keySendEnabled && resolution.Outcome != ResultInvoiceNotFound:
+	case !keySendEnabled && failResolution.Outcome != ResultInvoiceNotFound:
 		t.Fatal("expected invoice not found outcome")
 
-	case keySendEnabled && resolution.Outcome != ResultKeySendError:
+	case keySendEnabled && failResolution.Outcome != ResultKeySendError:
 		t.Fatal("expected keysend error")
 	}
 
@@ -676,15 +709,25 @@ func testKeySend(t *testing.T, keySendEnabled bool) {
 
 	// Expect a cancel resolution if keysend is disabled.
 	if !keySendEnabled {
-		if resolution.Outcome != ResultInvoiceNotFound {
+		failResolution, ok = resolution.(*HtlcFailResolution)
+		if !ok {
+			t.Fatalf("expected fail resolution, got: %T",
+				resolution)
+		}
+		if failResolution.Outcome != ResultInvoiceNotFound {
 			t.Fatal("expected keysend payment not to be accepted")
 		}
 		return
 	}
 
 	// Otherwise we expect no error and a settle resolution for the htlc.
-	if resolution.Preimage == nil || *resolution.Preimage != preimage {
-		t.Fatal("expected valid settle event")
+	settleResolution, ok := resolution.(*HtlcSettleResolution)
+	if !ok {
+		t.Fatalf("expected settle resolution, got: %T",
+			resolution)
+	}
+	if settleResolution.Preimage != preimage {
+		t.Fatalf("expected settle with matching preimage")
 	}
 
 	// We expect a new invoice notification to be sent out.
@@ -739,12 +782,14 @@ func TestMppPayment(t *testing.T) {
 	ctx.clock.SetTime(testTime.Add(30 * time.Second))
 
 	htlcResolution := (<-hodlChan1).(HtlcResolution)
-	if htlcResolution.Preimage != nil {
-		t.Fatal("expected cancel resolution")
+	failResolution, ok := htlcResolution.(*HtlcFailResolution)
+	if !ok {
+		t.Fatalf("expected fail resolution, got: %T",
+			resolution)
 	}
-	if htlcResolution.Outcome != ResultMppTimeout {
+	if failResolution.Outcome != ResultMppTimeout {
 		t.Fatalf("expected mpp timeout, got: %v",
-			htlcResolution.Outcome)
+			failResolution.Outcome)
 	}
 
 	// Send htlc 2.
@@ -771,12 +816,14 @@ func TestMppPayment(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if resolution == nil {
-		t.Fatal("expected a settle resolution")
+	settleResolution, ok := resolution.(*HtlcSettleResolution)
+	if !ok {
+		t.Fatalf("expected settle resolution, got: %T",
+			htlcResolution)
 	}
-	if resolution.Outcome != ResultSettled {
+	if settleResolution.Outcome != ResultSettled {
 		t.Fatalf("expected result settled, got: %v",
-			resolution.Outcome)
+			settleResolution.Outcome)
 	}
 
 	// Check that settled amount is equal to the sum of values of the htlcs

invoices/resolution.go

@@ -0,0 +1,124 @@
+package invoices
+
+import (
+	"time"
+
+	"github.com/lightningnetwork/lnd/channeldb"
+	"github.com/lightningnetwork/lnd/lntypes"
+)
+
+// HtlcResolution describes how an htlc should be resolved.
+type HtlcResolution interface {
+	// CircuitKey returns the circuit key for the htlc that we have a
+	// resolution for.
+	CircuitKey() channeldb.CircuitKey
+}
+
+// HtlcFailResolution is an implementation of the HtlcResolution interface
+// which is returned when a htlc is failed.
+type HtlcFailResolution struct {
+	// circuitKey is the key of the htlc for which we have a resolution.
+	circuitKey channeldb.CircuitKey
+
+	// AcceptHeight is the original height at which the htlc was accepted.
+	AcceptHeight int32
+
+	// outcome indicates the outcome of the invoice registry update.
+	Outcome ResolutionResult
+}
+
+// NewFailResolution returns a htlc failure resolution.
+func NewFailResolution(key channeldb.CircuitKey,
+	acceptHeight int32, outcome ResolutionResult) *HtlcFailResolution {
+
+	return &HtlcFailResolution{
+		circuitKey:   key,
+		AcceptHeight: acceptHeight,
+		Outcome:      outcome,
+	}
+}
+
+// CircuitKey returns the circuit key for the htlc that we have a
+// resolution for.
+//
+// Note: it is part of the HtlcResolution interface.
+func (f *HtlcFailResolution) CircuitKey() channeldb.CircuitKey {
+	return f.circuitKey
+}
+
+// HtlcSettleResolution is an implementation of the HtlcResolution interface
+// which is returned when a htlc is settled.
+type HtlcSettleResolution struct {
+	// Preimage is the htlc preimage. Its value is nil in case of a cancel.
+	Preimage lntypes.Preimage
+
+	// circuitKey is the key of the htlc for which we have a resolution.
+	circuitKey channeldb.CircuitKey
+
+	// acceptHeight is the original height at which the htlc was accepted.
+	AcceptHeight int32
+
+	// Outcome indicates the outcome of the invoice registry update.
+	Outcome ResolutionResult
+}
+
+// NewSettleResolution returns a htlc resolution which is associated with a
+// settle.
+func NewSettleResolution(preimage lntypes.Preimage, key channeldb.CircuitKey,
+	acceptHeight int32, outcome ResolutionResult) *HtlcSettleResolution {
+
+	return &HtlcSettleResolution{
+		Preimage:     preimage,
+		circuitKey:   key,
+		AcceptHeight: acceptHeight,
+		Outcome:      outcome,
+	}
+}
+
+// CircuitKey returns the circuit key for the htlc that we have a
+// resolution for.
+//
+// Note: it is part of the HtlcResolution interface.
+func (s *HtlcSettleResolution) CircuitKey() channeldb.CircuitKey {
+	return s.circuitKey
+}
+
+// htlcAcceptResolution is an implementation of the HtlcResolution interface
+// which is returned when a htlc is accepted. This struct is not exported
+// because the codebase uses a nil resolution to indicate that a htlc was
+// accepted. This struct is used internally in the invoice registry to
+// surface accept resolution results. When an invoice update returns an
+// acceptResolution, a nil resolution should be surfaced.
+type htlcAcceptResolution struct {
+	// circuitKey is the key of the htlc for which we have a resolution.
+	circuitKey channeldb.CircuitKey
+
+	// autoRelease signals that the htlc should be automatically released
+	// after a timeout.
+	autoRelease bool
+
+	// acceptTime is the time at which this htlc was accepted.
+	acceptTime time.Time
+
+	// outcome indicates the outcome of the invoice registry update.
+	outcome ResolutionResult
+}
+
+// newAcceptResolution returns a htlc resolution which is associated with a
+// htlc accept.
+func newAcceptResolution(key channeldb.CircuitKey,
+	outcome ResolutionResult) *htlcAcceptResolution {
+
+	return &htlcAcceptResolution{
+		circuitKey: key,
+		outcome:    outcome,
+	}
+}
+
+// CircuitKey returns the circuit key for the htlc that we have a
+// resolution for.
+//
+// Note: it is part of the HtlcResolution interface.
+func (a *htlcAcceptResolution) CircuitKey() channeldb.CircuitKey {
+	return a.circuitKey
+}

invoices/update.go

@@ -186,26 +186,53 @@ func (i *invoiceUpdateCtx) log(s string) {
 		i.hash[:], s, i.amtPaid, i.expiry, i.circuitKey, i.mpp)
 }
 
+// failRes is a helper function which creates a failure resolution with
+// the information contained in the invoiceUpdateCtx and the outcome provided.
+func (i invoiceUpdateCtx) failRes(outcome ResolutionResult) *HtlcFailResolution {
+	return NewFailResolution(i.circuitKey, i.currentHeight, outcome)
+}
+
+// settleRes is a helper function which creates a settle resolution with
+// the information contained in the invoiceUpdateCtx and the preimage and
+// outcome provided.
+func (i invoiceUpdateCtx) settleRes(preimage lntypes.Preimage,
+	outcome ResolutionResult) *HtlcSettleResolution {
+
+	return NewSettleResolution(
+		preimage, i.circuitKey, i.currentHeight, outcome,
+	)
+}
+
+// acceptRes is a helper function which creates an accept resolution with
+// the information contained in the invoiceUpdateCtx and the outcome provided.
+func (i invoiceUpdateCtx) acceptRes(outcome ResolutionResult) *htlcAcceptResolution {
+	return newAcceptResolution(i.circuitKey, outcome)
+}
+
 // updateInvoice is a callback for DB.UpdateInvoice that contains the invoice
-// settlement logic.
+// settlement logic. It returns a hltc resolution that indicates what the
+// outcome of the update was.
 func updateInvoice(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) (
-	*channeldb.InvoiceUpdateDesc, ResolutionResult, error) {
+	*channeldb.InvoiceUpdateDesc, HtlcResolution, error) {
 
 	// Don't update the invoice when this is a replayed htlc.
 	htlc, ok := inv.Htlcs[ctx.circuitKey]
 	if ok {
 		switch htlc.State {
 		case channeldb.HtlcStateCanceled:
-			return nil, ResultReplayToCanceled, nil
+			return nil, ctx.failRes(ResultReplayToCanceled), nil
 
 		case channeldb.HtlcStateAccepted:
-			return nil, ResultReplayToAccepted, nil
+			return nil, ctx.acceptRes(ResultReplayToAccepted), nil
 
 		case channeldb.HtlcStateSettled:
-			return nil, ResultReplayToSettled, nil
+			return nil, ctx.settleRes(
+				inv.Terms.PaymentPreimage,
+				ResultReplayToSettled,
+			), nil
 
 		default:
-			return nil, 0, errors.New("unknown htlc state")
+			return nil, nil, errors.New("unknown htlc state")
 		}
 	}
 
@@ -218,8 +245,9 @@ func updateInvoice(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) (
 
 // updateMpp is a callback for DB.UpdateInvoice that contains the invoice
 // settlement logic for mpp payments.
-func updateMpp(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) (
-	*channeldb.InvoiceUpdateDesc, ResolutionResult, error) {
+func updateMpp(ctx *invoiceUpdateCtx,
+	inv *channeldb.Invoice) (*channeldb.InvoiceUpdateDesc,
+	HtlcResolution, error) {
 
 	// Start building the accept descriptor.
 	acceptDesc := &channeldb.HtlcAcceptDesc{
@@ -235,23 +263,23 @@ func updateMpp(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) (
 	// Because non-mpp payments don't have a payment address, this is needed
 	// to thwart probing.
 	if inv.State != channeldb.ContractOpen {
-		return nil, ResultInvoiceNotOpen, nil
+		return nil, ctx.failRes(ResultInvoiceNotOpen), nil
 	}
 
 	// Check the payment address that authorizes the payment.
 	if ctx.mpp.PaymentAddr() != inv.Terms.PaymentAddr {
-		return nil, ResultAddressMismatch, nil
+		return nil, ctx.failRes(ResultAddressMismatch), nil
 	}
 
 	// Don't accept zero-valued sets.
 	if ctx.mpp.TotalMsat() == 0 {
-		return nil, ResultHtlcSetTotalTooLow, nil
+		return nil, ctx.failRes(ResultHtlcSetTotalTooLow), nil
 	}
 
 	// Check that the total amt of the htlc set is high enough. In case this
 	// is a zero-valued invoice, it will always be enough.
 	if ctx.mpp.TotalMsat() < inv.Terms.Value {
-		return nil, ResultHtlcSetTotalTooLow, nil
+		return nil, ctx.failRes(ResultHtlcSetTotalTooLow), nil
 	}
 
 	// Check whether total amt matches other htlcs in the set.
@@ -265,7 +293,7 @@ func updateMpp(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) (
 		}
 
 		if ctx.mpp.TotalMsat() != htlc.MppTotalAmt {
-			return nil, ResultHtlcSetTotalMismatch, nil
+			return nil, ctx.failRes(ResultHtlcSetTotalMismatch), nil
 		}
 
 		newSetTotal += htlc.Amt
@@ -276,16 +304,16 @@ func updateMpp(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) (
 
 	// Make sure the communicated set total isn't overpaid.
 	if newSetTotal > ctx.mpp.TotalMsat() {
-		return nil, ResultHtlcSetOverpayment, nil
+		return nil, ctx.failRes(ResultHtlcSetOverpayment), nil
 	}
 
 	// The invoice is still open. Check the expiry.
 	if ctx.expiry < uint32(ctx.currentHeight+ctx.finalCltvRejectDelta) {
-		return nil, ResultExpiryTooSoon, nil
+		return nil, ctx.failRes(ResultExpiryTooSoon), nil
 	}
 
 	if ctx.expiry < uint32(ctx.currentHeight+inv.Terms.FinalCltvDelta) {
-		return nil, ResultExpiryTooSoon, nil
+		return nil, ctx.failRes(ResultExpiryTooSoon), nil
 	}
 
 	// Record HTLC in the invoice database.
@@ -300,7 +328,7 @@ func updateMpp(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) (
 	// If the invoice cannot be settled yet, only record the htlc.
 	setComplete := newSetTotal == ctx.mpp.TotalMsat()
 	if !setComplete {
-		return &update, ResultPartialAccepted, nil
+		return &update, ctx.acceptRes(ResultPartialAccepted), nil
 	}
 
 	// Check to see if we can settle or this is an hold invoice and
@@ -310,7 +338,7 @@ func updateMpp(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) (
 		update.State = &channeldb.InvoiceStateUpdateDesc{
 			NewState: channeldb.ContractAccepted,
 		}
-		return &update, ResultAccepted, nil
+		return &update, ctx.acceptRes(ResultAccepted), nil
 	}
 
 	update.State = &channeldb.InvoiceStateUpdateDesc{
@@ -318,18 +346,20 @@ func updateMpp(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) (
 		Preimage: inv.Terms.PaymentPreimage,
 	}
 
-	return &update, ResultSettled, nil
+	return &update, ctx.settleRes(
+		inv.Terms.PaymentPreimage, ResultSettled,
+	), nil
 }
 
 // updateLegacy is a callback for DB.UpdateInvoice that contains the invoice
 // settlement logic for legacy payments.
-func updateLegacy(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) (
-	*channeldb.InvoiceUpdateDesc, ResolutionResult, error) {
+func updateLegacy(ctx *invoiceUpdateCtx,
+	inv *channeldb.Invoice) (*channeldb.InvoiceUpdateDesc, HtlcResolution, error) {
 
 	// If the invoice is already canceled, there is no further
 	// checking to do.
 	if inv.State == channeldb.ContractCanceled {
-		return nil, ResultInvoiceAlreadyCanceled, nil
+		return nil, ctx.failRes(ResultInvoiceAlreadyCanceled), nil
 	}
 
 	// If an invoice amount is specified, check that enough is paid. Also
@@ -337,7 +367,7 @@ func updateLegacy(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) (
 	// or accepted. In case this is a zero-valued invoice, it will always be
 	// enough.
 	if ctx.amtPaid < inv.Terms.Value {
-		return nil, ResultAmountTooLow, nil
+		return nil, ctx.failRes(ResultAmountTooLow), nil
 	}
 
 	// TODO(joostjager): Check invoice mpp required feature
@@ -350,17 +380,17 @@ func updateLegacy(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) (
 		if htlc.State == channeldb.HtlcStateAccepted &&
 			htlc.MppTotalAmt > 0 {
 
-			return nil, ResultMppInProgress, nil
+			return nil, ctx.failRes(ResultMppInProgress), nil
 		}
 	}
 
 	// The invoice is still open. Check the expiry.
 	if ctx.expiry < uint32(ctx.currentHeight+ctx.finalCltvRejectDelta) {
-		return nil, ResultExpiryTooSoon, nil
+		return nil, ctx.failRes(ResultExpiryTooSoon), nil
 	}
 
 	if ctx.expiry < uint32(ctx.currentHeight+inv.Terms.FinalCltvDelta) {
-		return nil, ResultExpiryTooSoon, nil
+		return nil, ctx.failRes(ResultExpiryTooSoon), nil
 	}
 
 	// Record HTLC in the invoice database.
@@ -381,10 +411,12 @@ func updateLegacy(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) (
 	// We do accept or settle the HTLC.
 	switch inv.State {
 	case channeldb.ContractAccepted:
-		return &update, ResultDuplicateToAccepted, nil
+		return &update, ctx.acceptRes(ResultDuplicateToAccepted), nil
 
 	case channeldb.ContractSettled:
-		return &update, ResultDuplicateToSettled, nil
+		return &update, ctx.settleRes(
+			inv.Terms.PaymentPreimage, ResultDuplicateToSettled,
+		), nil
 	}
 
 	// Check to see if we can settle or this is an hold invoice and we need
@@ -394,7 +426,8 @@ func updateLegacy(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) (
 		update.State = &channeldb.InvoiceStateUpdateDesc{
 			NewState: channeldb.ContractAccepted,
 		}
-		return &update, ResultAccepted, nil
+
+		return &update, ctx.acceptRes(ResultAccepted), nil
 	}
 
 	update.State = &channeldb.InvoiceStateUpdateDesc{
@@ -402,5 +435,7 @@ func updateLegacy(ctx *invoiceUpdateCtx, inv *channeldb.Invoice) (
 		Preimage: inv.Terms.PaymentPreimage,
 	}
 
-	return &update, ResultSettled, nil
+	return &update, ctx.settleRes(
+		inv.Terms.PaymentPreimage, ResultSettled,
+	), nil
 }