Merge pull request #3844 from carlaKC/htlcnotifier-1-detailedswitcherrors

invoiceregistry+htlcswitch: Introduce resolution types and add link errors

contractcourt/htlc_incoming_contest_resolver.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/binary"
 	"errors"
+	"fmt"
 	"io"
 
 	"github.com/btcsuite/btcutil"
@@ -172,7 +173,23 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) {
 	processHtlcResolution := func(e invoices.HtlcResolution) (
 		ContractResolver, error) {
 
-		if e.Preimage == nil {
+		// Take action based on the type of resolution we have
+		// received.
+		switch resolution := e.(type) {
+
+		// If the htlc resolution was a settle, apply the
+		// preimage and return a success resolver.
+		case *invoices.HtlcSettleResolution:
+			err := applyPreimage(resolution.Preimage)
+			if err != nil {
+				return nil, err
+			}
+
+			return &h.htlcSuccessResolver, nil
+
+		// If the htlc was failed, mark the htlc as
+		// resolved.
+		case *invoices.HtlcFailResolution:
 			log.Infof("%T(%v): Exit hop HTLC canceled "+
 				"(expiry=%v, height=%v), abandoning", h,
 				h.htlcResolution.ClaimOutpoint,
@@ -180,13 +197,13 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) {
 
 			h.resolved = true
 			return nil, h.Checkpoint(h)
-		}
 
-		if err := applyPreimage(*e.Preimage); err != nil {
+		// Error if the resolution type is unknown, we are only
-			return nil, err
+		// expecting settles and fails.
+		default:
+			return nil, fmt.Errorf("unknown resolution"+
+				" type: %v", e)
 		}
-
-		return &h.htlcSuccessResolver, nil
 	}
 
 	// Create a buffered hodl chan to prevent deadlock.
@@ -211,14 +228,29 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) {
 
 	defer h.Registry.HodlUnsubscribeAll(hodlChan)
 
-	// If the resolution is non-nil (indicating that a settle or cancel has
+	// Take action based on the resolution we received. If the htlc was
-	// occurred), and the invoice is known to the registry (indicating that
+	// settled, or a htlc for a known invoice failed we can resolve it
-	// the htlc is paying one of our invoices and is not a forward), try to
+	// directly. If the resolution is nil, the htlc was neither accepted
-	// resolve it directly.
+	// nor failed, so we cannot take action yet.
-	if resolution != nil &&
+	switch res := resolution.(type) {
-		resolution.Outcome != invoices.ResultInvoiceNotFound {
+	case *invoices.HtlcFailResolution:
+		// In the case where the htlc failed, but the invoice was known
+		// to the registry, we can directly resolve the htlc.
+		if res.Outcome != invoices.ResultInvoiceNotFound {
+			return processHtlcResolution(resolution)
+		}
 
-		return processHtlcResolution(*resolution)
+	// If we settled the htlc, we can resolve it.
+	case *invoices.HtlcSettleResolution:
+		return processHtlcResolution(resolution)
+
+	// If the resolution is nil, the htlc was neither settled nor failed so
+	// we cannot take action at present.
+	case nil:
+
+	default:
+		return nil, fmt.Errorf("unknown htlc resolution type: %T",
+			resolution)
 	}
 
 	// With the epochs and preimage subscriptions initialized, we'll query
@@ -256,7 +288,6 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) {
 
 		case hodlItem := <-hodlChan:
 			htlcResolution := hodlItem.(invoices.HtlcResolution)
-
 			return processHtlcResolution(htlcResolution)
 
 		case newBlock, ok := <-blockEpochs.Epochs:

contractcourt/htlc_incoming_resolver_test.go

@@ -35,7 +35,7 @@ func TestHtlcIncomingResolverFwdPreimageKnown(t *testing.T) {
 	defer timeout(t)()
 
 	ctx := newIncomingResolverTestContext(t)
-	ctx.registry.notifyResolution = invoices.NewFailureResolution(
+	ctx.registry.notifyResolution = invoices.NewFailResolution(
 		testResCircuitKey, testHtlcExpiry,
 		invoices.ResultInvoiceNotFound,
 	)
@@ -52,7 +52,7 @@ func TestHtlcIncomingResolverFwdContestedSuccess(t *testing.T) {
 	defer timeout(t)()
 
 	ctx := newIncomingResolverTestContext(t)
-	ctx.registry.notifyResolution = invoices.NewFailureResolution(
+	ctx.registry.notifyResolution = invoices.NewFailResolution(
 		testResCircuitKey, testHtlcExpiry,
 		invoices.ResultInvoiceNotFound,
 	)
@@ -72,7 +72,7 @@ func TestHtlcIncomingResolverFwdContestedTimeout(t *testing.T) {
 	defer timeout(t)()
 
 	ctx := newIncomingResolverTestContext(t)
-	ctx.registry.notifyResolution = invoices.NewFailureResolution(
+	ctx.registry.notifyResolution = invoices.NewFailResolution(
 		testResCircuitKey, testHtlcExpiry,
 		invoices.ResultInvoiceNotFound,
 	)
@@ -91,7 +91,7 @@ func TestHtlcIncomingResolverFwdTimeout(t *testing.T) {
 	defer timeout(t)()
 
 	ctx := newIncomingResolverTestContext(t)
-	ctx.registry.notifyResolution = invoices.NewFailureResolution(
+	ctx.registry.notifyResolution = invoices.NewFailResolution(
 		testResCircuitKey, testHtlcExpiry,
 		invoices.ResultInvoiceNotFound,
 	)
@@ -139,7 +139,7 @@ func TestHtlcIncomingResolverExitCancel(t *testing.T) {
 	defer timeout(t)()
 
 	ctx := newIncomingResolverTestContext(t)
-	ctx.registry.notifyResolution = invoices.NewFailureResolution(
+	ctx.registry.notifyResolution = invoices.NewFailResolution(
 		testResCircuitKey, testAcceptHeight,
 		invoices.ResultInvoiceAlreadyCanceled,
 	)
@@ -158,7 +158,7 @@ func TestHtlcIncomingResolverExitSettleHodl(t *testing.T) {
 	ctx.resolve()
 
 	notifyData := <-ctx.registry.notifyChan
-	notifyData.hodlChan <- *invoices.NewSettleResolution(
+	notifyData.hodlChan <- invoices.NewSettleResolution(
 		testResPreimage, testResCircuitKey, testAcceptHeight,
 		invoices.ResultSettled,
 	)
@@ -187,7 +187,7 @@ func TestHtlcIncomingResolverExitCancelHodl(t *testing.T) {
 	ctx := newIncomingResolverTestContext(t)
 	ctx.resolve()
 	notifyData := <-ctx.registry.notifyChan
-	notifyData.hodlChan <- *invoices.NewFailureResolution(
+	notifyData.hodlChan <- invoices.NewFailResolution(
 		testResCircuitKey, testAcceptHeight, invoices.ResultCanceled,
 	)
 

contractcourt/interfaces.go

@@ -27,7 +27,7 @@ type Registry interface {
 	NotifyExitHopHtlc(payHash lntypes.Hash, paidAmount lnwire.MilliSatoshi,
 		expiry uint32, currentHeight int32,
 		circuitKey channeldb.CircuitKey, hodlChan chan<- interface{},
-		payload invoices.Payload) (*invoices.HtlcResolution, error)
+		payload invoices.Payload) (invoices.HtlcResolution, error)
 
 	// HodlUnsubscribeAll unsubscribes from all htlc resolutions.
 	HodlUnsubscribeAll(subscriber chan<- interface{})

contractcourt/mock_registry_test.go

@@ -18,13 +18,13 @@ type notifyExitHopData struct {
 type mockRegistry struct {
 	notifyChan       chan notifyExitHopData
 	notifyErr        error
-	notifyResolution *invoices.HtlcResolution
+	notifyResolution invoices.HtlcResolution
 }
 
 func (r *mockRegistry) NotifyExitHopHtlc(payHash lntypes.Hash,
 	paidAmount lnwire.MilliSatoshi, expiry uint32, currentHeight int32,
 	circuitKey channeldb.CircuitKey, hodlChan chan<- interface{},
-	payload invoices.Payload) (*invoices.HtlcResolution, error) {
+	payload invoices.Payload) (invoices.HtlcResolution, error) {
 
 	r.notifyChan <- notifyExitHopData{
 		hodlChan:      hodlChan,

htlcswitch/failure.go

@@ -72,11 +72,11 @@ func (l *LinkError) WireMessage() lnwire.FailureMessage {
 func (l *LinkError) Error() string {
 	// If the link error has no failure detail, return the wire message's
 	// error.
-	if l.FailureDetail == FailureDetailNone {
+	if l.FailureDetail == nil {
 		return l.msg.Error()
 	}
 
-	return fmt.Sprintf("%v: %v", l.msg.Error(), l.FailureDetail)
+	return l.FailureDetail.FailureString()
 }
 
 // ForwardingError wraps an lnwire.FailureMessage in a struct that also

htlcswitch/failure_detail.go

@@ -1,65 +1,96 @@
 package htlcswitch
 
-// FailureDetail is an enum which is used to enrich failures with
+// FailureDetail is an interface implemented by failures that occur on
-// additional information.
+// our incoming or outgoing link, or within the switch itself.
-type FailureDetail int
+type FailureDetail interface {
+	// FailureString returns the string representation of a failure
+	// detail.
+	FailureString() string
+}
+
+// OutgoingFailure is an enum which is used to enrich failures which occur in
+// the switch or on our outgoing link with additional metadata.
+type OutgoingFailure int
 
 const (
-	// FailureDetailNone is returned when the wire message contains
+	// OutgoingFailureNone is returned when the wire message contains
 	// sufficient information.
-	FailureDetailNone = iota
+	OutgoingFailureNone OutgoingFailure = iota
 
-	// FailureDetailOnionDecode indicates that we could not decode an
+	// OutgoingFailureDecodeError indicates that we could not decode the
-	// onion error.
+	// failure reason provided for a failed payment.
-	FailureDetailOnionDecode
+	OutgoingFailureDecodeError
 
-	// FailureDetailLinkNotEligible indicates that a routing attempt was
+	// OutgoingFailureLinkNotEligible indicates that a routing attempt was
 	// made over a link that is not eligible for routing.
-	FailureDetailLinkNotEligible
+	OutgoingFailureLinkNotEligible
 
-	// FailureDetailOnChainTimeout indicates that a payment had to be timed
+	// OutgoingFailureOnChainTimeout indicates that a payment had to be
-	// out on chain before it got past the first hop by us or the remote
+	// timed out on chain before it got past the first hop by us or the
-	// party.
+	// remote party.
-	FailureDetailOnChainTimeout
+	OutgoingFailureOnChainTimeout
 
-	// FailureDetailHTLCExceedsMax is returned when a htlc exceeds our
+	// OutgoingFailureHTLCExceedsMax is returned when a htlc exceeds our
 	// policy's maximum htlc amount.
-	FailureDetailHTLCExceedsMax
+	OutgoingFailureHTLCExceedsMax
 
-	// FailureDetailInsufficientBalance is returned when we cannot route a
+	// OutgoingFailureInsufficientBalance is returned when we cannot route a
 	// htlc due to insufficient outgoing capacity.
-	FailureDetailInsufficientBalance
+	OutgoingFailureInsufficientBalance
 
-	// FailureDetailCircularRoute is returned when an attempt is made
+	// OutgoingFailureCircularRoute is returned when an attempt is made
 	// to forward a htlc through our node which arrives and leaves on the
 	// same channel.
-	FailureDetailCircularRoute
+	OutgoingFailureCircularRoute
+
+	// OutgoingFailureIncompleteForward is returned when we cancel an incomplete
+	// forward.
+	OutgoingFailureIncompleteForward
+
+	// OutgoingFailureDownstreamHtlcAdd is returned when we fail to add a
+	// downstream htlc to our outgoing link.
+	OutgoingFailureDownstreamHtlcAdd
+
+	// OutgoingFailureForwardsDisabled is returned when the switch is
+	// configured to disallow forwards.
+	OutgoingFailureForwardsDisabled
 )
 
-// String returns the string representation of a failure detail.
+// FailureString returns the string representation of a failure detail.
-func (fd FailureDetail) String() string {
+//
+// Note: it is part of the FailureDetail interface.
+func (fd OutgoingFailure) FailureString() string {
 	switch fd {
-	case FailureDetailNone:
+	case OutgoingFailureNone:
 		return "no failure detail"
 
-	case FailureDetailOnionDecode:
+	case OutgoingFailureDecodeError:
-		return "could not decode onion"
+		return "could not decode wire failure"
 
-	case FailureDetailLinkNotEligible:
+	case OutgoingFailureLinkNotEligible:
 		return "link not eligible"
 
-	case FailureDetailOnChainTimeout:
+	case OutgoingFailureOnChainTimeout:
 		return "payment was resolved on-chain, then canceled back"
 
-	case FailureDetailHTLCExceedsMax:
+	case OutgoingFailureHTLCExceedsMax:
 		return "htlc exceeds maximum policy amount"
 
-	case FailureDetailInsufficientBalance:
+	case OutgoingFailureInsufficientBalance:
 		return "insufficient bandwidth to route htlc"
 
-	case FailureDetailCircularRoute:
+	case OutgoingFailureCircularRoute:
 		return "same incoming and outgoing channel"
 
+	case OutgoingFailureIncompleteForward:
+		return "failed after detecting incomplete forward"
+
+	case OutgoingFailureDownstreamHtlcAdd:
+		return "could not add downstream htlc"
+
+	case OutgoingFailureForwardsDisabled:
+		return "node configured to disallow forwards"
+
 	default:
 		return "unknown failure detail"
 	}

htlcswitch/interfaces.go

@@ -27,7 +27,7 @@ type InvoiceDatabase interface {
 	NotifyExitHopHtlc(payHash lntypes.Hash, paidAmount lnwire.MilliSatoshi,
 		expiry uint32, currentHeight int32,
 		circuitKey channeldb.CircuitKey, hodlChan chan<- interface{},
-		payload invoices.Payload) (*invoices.HtlcResolution, error)
+		payload invoices.Payload) (invoices.HtlcResolution, error)
 
 	// CancelInvoice attempts to cancel the invoice corresponding to the
 	// passed payment hash.

htlcswitch/link.go

@@ -1158,7 +1158,7 @@ loop:
 	for {
 		// Lookup all hodl htlcs that can be failed or settled with this event.
 		// The hodl htlc must be present in the map.
-		circuitKey := htlcResolution.CircuitKey
+		circuitKey := htlcResolution.CircuitKey()
 		hodlHtlc, ok := l.hodlMap[circuitKey]
 		if !ok {
 			return fmt.Errorf("hodl htlc not found: %v", circuitKey)
@@ -1193,49 +1193,68 @@ loop:
 func (l *channelLink) processHtlcResolution(resolution invoices.HtlcResolution,
 	htlc hodlHtlc) error {
 
-	circuitKey := resolution.CircuitKey
+	circuitKey := resolution.CircuitKey()
 
-	// Determine required action for the resolution. If the event's preimage is
+	// Determine required action for the resolution based on the type of
-	// non-nil, the htlc must be settled. Otherwise, it should be canceled.
+	// resolution we have received.
-	if resolution.Preimage != nil {
+	switch res := resolution.(type) {
-		l.log.Debugf("received settle resolution for %v", circuitKey)
+	// Settle htlcs that returned a settle resolution using the preimage
+	// in the resolution.
+	case *invoices.HtlcSettleResolution:
+		l.log.Debugf("received settle resolution for %v"+
+			"with outcome: %v", circuitKey, res.Outcome)
 
 		return l.settleHTLC(
-			*resolution.Preimage, htlc.pd.HtlcIndex,
+			res.Preimage, htlc.pd.HtlcIndex,
 			htlc.pd.SourceRef,
 		)
+
+	// For htlc failures, we get the relevant failure message based
+	// on the failure resolution and then fail the htlc.
+	case *invoices.HtlcFailResolution:
+		l.log.Debugf("received cancel resolution for "+
+			"%v with outcome: %v", circuitKey, res.Outcome)
+
+		// Get the lnwire failure message based on the resolution
+		// result.
+		failure := getResolutionFailure(res, htlc.pd.Amount)
+
+		l.sendHTLCError(
+			htlc.pd.HtlcIndex, failure,
+			htlc.obfuscator, htlc.pd.SourceRef,
+		)
+		return nil
+
+	// Fail if we do not get a settle of fail resolution, since we
+	// are only expecting to handle settles and fails.
+	default:
+		return fmt.Errorf("unknown htlc resolution type: %T",
+			resolution)
 	}
-
-	l.log.Debugf("received cancel resolution for %v with outcome: %v",
-		circuitKey, resolution.Outcome)
-
-	// Get the lnwire failure message based on the resolution result.
-	failure := getResolutionFailure(resolution, htlc.pd.Amount)
-
-	l.sendHTLCError(
-		htlc.pd.HtlcIndex, failure, htlc.obfuscator,
-		htlc.pd.SourceRef,
-	)
-	return nil
 }
 
 // getResolutionFailure returns the wire message that a htlc resolution should
 // be failed with.
-func getResolutionFailure(resolution invoices.HtlcResolution,
+func getResolutionFailure(resolution *invoices.HtlcFailResolution,
-	amount lnwire.MilliSatoshi) lnwire.FailureMessage {
+	amount lnwire.MilliSatoshi) *LinkError {
 
-	// If the resolution has been resolved as part of a MPP timeout, we need
+	// If the resolution has been resolved as part of a MPP timeout,
-	// to fail the htlc with lnwire.FailMppTimeout.
+	// we need to fail the htlc with lnwire.FailMppTimeout.
 	if resolution.Outcome == invoices.ResultMppTimeout {
-		return &lnwire.FailMPPTimeout{}
+		return NewDetailedLinkError(
+			&lnwire.FailMPPTimeout{}, resolution.Outcome,
+		)
 	}
 
-	// If the htlc is not a MPP timeout, we fail it with FailIncorrectDetails
+	// If the htlc is not a MPP timeout, we fail it with
-	// This covers hodl cancels (which return it to avoid leaking information
+	// FailIncorrectDetails. This error is sent for invoice payment
-	// and other invoice failures such as underpayment or expiry too soon.
+	// failures such as underpayment/ expiry too soon and hodl invoices
-	return lnwire.NewFailIncorrectDetails(
+	// (which return FailIncorrectDetails to avoid leaking information).
+	incorrectDetails := lnwire.NewFailIncorrectDetails(
 		amount, uint32(resolution.AcceptHeight),
 	)
+
+	return NewDetailedLinkError(incorrectDetails, resolution.Outcome)
 }
 
 // randomFeeUpdateTimeout returns a random timeout between the bounds defined
@@ -1299,6 +1318,10 @@ func (l *channelLink) handleDownStreamPkt(pkt *htlcPacket, isReProcess bool) {
 					reason       lnwire.OpaqueReason
 				)
 
+				// Create a temporary channel failure which we
+				// will send back to our peer if this is a
+				// forward, or report to the user if the failed
+				// payment was locally initiated.
 				failure := l.createFailureWithUpdate(
 					func(upd *lnwire.ChannelUpdate) lnwire.FailureMessage {
 						return lnwire.NewTemporaryChannelFailure(
@@ -1307,28 +1330,43 @@ func (l *channelLink) handleDownStreamPkt(pkt *htlcPacket, isReProcess bool) {
 					},
 				)
 
-				// Encrypt the error back to the source unless
+				// If the payment was locally initiated (which
-				// the payment was generated locally.
+				// is indicated by a nil obfuscator), we do
+				// not need to encrypt it back to the sender.
 				if pkt.obfuscator == nil {
 					var b bytes.Buffer
 					err := lnwire.EncodeFailure(&b, failure, 0)
 					if err != nil {
-						l.log.Errorf("unable to encode failure: %v", err)
+						l.log.Errorf("unable to "+
+							"encode failure: %v", err)
 						l.mailBox.AckPacket(pkt.inKey())
 						return
 					}
 					reason = lnwire.OpaqueReason(b.Bytes())
 					localFailure = true
 				} else {
+					// If the packet is part of a forward,
+					// (identified by a non-nil obfuscator)
+					// we need to encrypt the error back to
+					// the source.
 					var err error
 					reason, err = pkt.obfuscator.EncryptFirstHop(failure)
 					if err != nil {
-						l.log.Errorf("unable to obfuscate error: %v", err)
+						l.log.Errorf("unable to "+
+							"obfuscate error: %v", err)
 						l.mailBox.AckPacket(pkt.inKey())
 						return
 					}
 				}
 
+				// Create a link error containing the temporary
+				// channel failure and a detail which indicates
+				// the we failed to add the htlc.
+				linkError := NewDetailedLinkError(
+					failure,
+					OutgoingFailureDownstreamHtlcAdd,
+				)
+
 				failPkt := &htlcPacket{
 					incomingChanID: pkt.incomingChanID,
 					incomingHTLCID: pkt.incomingHTLCID,
@@ -1336,6 +1374,7 @@ func (l *channelLink) handleDownStreamPkt(pkt *htlcPacket, isReProcess bool) {
 					sourceRef:      pkt.sourceRef,
 					hasSource:      true,
 					localFailure:   localFailure,
+					linkFailure:    linkError,
 					htlc: &lnwire.UpdateFailHTLC{
 						Reason: reason,
 					},
@@ -2269,7 +2308,7 @@ func (l *channelLink) canSendHtlc(policy ForwardingPolicy,
 				return lnwire.NewTemporaryChannelFailure(upd)
 			},
 		)
-		return NewDetailedLinkError(failure, FailureDetailHTLCExceedsMax)
+		return NewDetailedLinkError(failure, OutgoingFailureHTLCExceedsMax)
 	}
 
 	// We want to avoid offering an HTLC which will expire in the near
@@ -2304,7 +2343,7 @@ func (l *channelLink) canSendHtlc(policy ForwardingPolicy,
 			},
 		)
 		return NewDetailedLinkError(
-			failure, FailureDetailInsufficientBalance,
+			failure, OutgoingFailureInsufficientBalance,
 		)
 	}
 
@@ -2446,7 +2485,9 @@ func (l *channelLink) processRemoteSettleFails(fwdPkg *channeldb.FwdPkg,
 			}
 
 			// Fetch the reason the HTLC was canceled so we can
-			// continue to propagate it.
+			// continue to propagate it. This failure originated
+			// from another node, so the linkFailure field is not
+			// set on the packet.
 			failPacket := &htlcPacket{
 				outgoingChanID: l.ShortChanID(),
 				outgoingHTLCID: pd.ParentIndex,
@@ -2613,9 +2654,10 @@ func (l *channelLink) processRemoteAdds(fwdPkg *channeldb.FwdPkg,
 			// for TLV payloads that also supports injecting invalid
 			// payloads. Deferring this non-trival effort till a
 			// later date
+			failure := lnwire.NewInvalidOnionPayload(failedType, 0)
 			l.sendHTLCError(
 				pd.HtlcIndex,
-				lnwire.NewInvalidOnionPayload(failedType, 0),
+				NewLinkError(failure),
 				obfuscator, pd.SourceRef,
 			)
 
@@ -2729,7 +2771,10 @@ func (l *channelLink) processRemoteAdds(fwdPkg *channeldb.FwdPkg,
 				)
 
 				l.sendHTLCError(
-					pd.HtlcIndex, failure, obfuscator, pd.SourceRef,
+					pd.HtlcIndex,
+					NewLinkError(failure),
+					obfuscator,
+					pd.SourceRef,
 				)
 				continue
 			}
@@ -2812,7 +2857,9 @@ func (l *channelLink) processExitHop(pd *lnwallet.PaymentDescriptor,
 			"value: expected %v, got %v", pd.RHash,
 			pd.Amount, fwdInfo.AmountToForward)
 
-		failure := lnwire.NewFinalIncorrectHtlcAmount(pd.Amount)
+		failure := NewLinkError(
+			lnwire.NewFinalIncorrectHtlcAmount(pd.Amount),
+		)
 		l.sendHTLCError(pd.HtlcIndex, failure, obfuscator, pd.SourceRef)
 
 		return nil
@@ -2825,7 +2872,9 @@ func (l *channelLink) processExitHop(pd *lnwallet.PaymentDescriptor,
 			"time-lock: expected %v, got %v",
 			pd.RHash[:], pd.Timeout, fwdInfo.OutgoingCTLV)
 
-		failure := lnwire.NewFinalIncorrectCltvExpiry(pd.Timeout)
+		failure := NewLinkError(
+			lnwire.NewFinalIncorrectCltvExpiry(pd.Timeout),
+		)
 		l.sendHTLCError(pd.HtlcIndex, failure, obfuscator, pd.SourceRef)
 
 		return nil
@@ -2863,7 +2912,7 @@ func (l *channelLink) processExitHop(pd *lnwallet.PaymentDescriptor,
 	}
 
 	// Process the received resolution.
-	return l.processHtlcResolution(*event, htlc)
+	return l.processHtlcResolution(event, htlc)
 }
 
 // settleHTLC settles the HTLC on the channel.
@@ -2942,10 +2991,10 @@ func (l *channelLink) handleBatchFwdErrs(errChan chan error) {
 
 // sendHTLCError functions cancels HTLC and send cancel message back to the
 // peer from which HTLC was received.
-func (l *channelLink) sendHTLCError(htlcIndex uint64, failure lnwire.FailureMessage,
+func (l *channelLink) sendHTLCError(htlcIndex uint64, failure *LinkError,
 	e hop.ErrorEncrypter, sourceRef *channeldb.AddRef) {
 
-	reason, err := e.EncryptFirstHop(failure)
+	reason, err := e.EncryptFirstHop(failure.WireMessage())
 	if err != nil {
 		l.log.Errorf("unable to obfuscate error: %v", err)
 		return

htlcswitch/mock.go

@@ -816,7 +816,7 @@ func (i *mockInvoiceRegistry) SettleHodlInvoice(preimage lntypes.Preimage) error
 func (i *mockInvoiceRegistry) NotifyExitHopHtlc(rhash lntypes.Hash,
 	amt lnwire.MilliSatoshi, expiry uint32, currentHeight int32,
 	circuitKey channeldb.CircuitKey, hodlChan chan<- interface{},
-	payload invoices.Payload) (*invoices.HtlcResolution, error) {
+	payload invoices.Payload) (invoices.HtlcResolution, error) {
 
 	event, err := i.registry.NotifyExitHopHtlc(
 		rhash, amt, expiry, currentHeight, circuitKey, hodlChan,

htlcswitch/packet.go

@@ -54,6 +54,11 @@ type htlcPacket struct {
 	// encrypted with any shared secret.
 	localFailure bool
 
+	// linkFailure is non-nil for htlcs that fail at our node. This may
+	// occur for our own payments which fail on the outgoing link,
+	// or for forwards which fail in the switch or on the outgoing link.
+	linkFailure *LinkError
+
 	// convertedError is set to true if this is an HTLC fail that was
 	// created using an UpdateFailMalformedHTLC from the remote party. If
 	// this is true, then when forwarding this failure packet, we'll need

htlcswitch/switch.go

@@ -45,11 +45,6 @@ var (
 	// through the switch and is locked into another commitment txn.
 	ErrDuplicateAdd = errors.New("duplicate add HTLC detected")
 
-	// ErrIncompleteForward is used when an htlc was already forwarded
-	// through the switch, but did not get locked into another commitment
-	// txn.
-	ErrIncompleteForward = errors.New("incomplete forward detected")
-
 	// ErrUnknownErrorDecryptor signals that we were unable to locate the
 	// error decryptor for this payment. This is likely due to restarting
 	// the daemon.
@@ -496,9 +491,12 @@ func (s *Switch) forward(packet *htlcPacket) error {
 			} else {
 				failure = lnwire.NewTemporaryChannelFailure(update)
 			}
-			addErr := ErrIncompleteForward
 
-			return s.failAddPacket(packet, failure, addErr)
+			linkError := NewDetailedLinkError(
+				failure, OutgoingFailureIncompleteForward,
+			)
+
+			return s.failAddPacket(packet, linkError)
 		}
 
 		packet.circuit = circuit
@@ -647,14 +645,14 @@ func (s *Switch) ForwardPackets(linkQuit chan struct{},
 		} else {
 			failure = lnwire.NewTemporaryChannelFailure(update)
 		}
+		linkError := NewDetailedLinkError(
+			failure, OutgoingFailureIncompleteForward,
+		)
 
 		for _, packet := range failedPackets {
-			addErr := errors.New("failing packet after " +
-				"detecting incomplete forward")
-
 			// We don't handle the error here since this method
 			// always returns an error.
-			s.failAddPacket(packet, failure, addErr)
+			_ = s.failAddPacket(packet, linkError)
 		}
 	}
 
@@ -771,7 +769,7 @@ func (s *Switch) handleLocalDispatch(pkt *htlcPacket) error {
 			// will be returned back to the router.
 			return NewDetailedLinkError(
 				lnwire.NewTemporaryChannelFailure(nil),
-				FailureDetailLinkNotEligible,
+				OutgoingFailureLinkNotEligible,
 			)
 		}
 
@@ -919,12 +917,12 @@ func (s *Switch) parseFailedPayment(deobfuscator ErrorDecrypter,
 				// need to apply an update here since it goes
 				// directly to the router.
 				lnwire.NewTemporaryChannelFailure(nil),
-				FailureDetailOnionDecode,
+				OutgoingFailureDecodeError,
 			)
 
 			log.Errorf("%v: (hash=%v, pid=%d): %v",
-				linkError.FailureDetail, paymentHash, paymentID,
+				linkError.FailureDetail.FailureString(),
-				err)
+				paymentHash, paymentID, err)
 
 			return linkError
 		}
@@ -939,10 +937,11 @@ func (s *Switch) parseFailedPayment(deobfuscator ErrorDecrypter,
 	case isResolution && htlc.Reason == nil:
 		linkError := NewDetailedLinkError(
 			&lnwire.FailPermanentChannelFailure{},
-			FailureDetailOnChainTimeout,
+			OutgoingFailureOnChainTimeout,
 		)
 
-		log.Info("%v: hash=%v, pid=%d", linkError.FailureDetail,
+		log.Info("%v: hash=%v, pid=%d",
+			linkError.FailureDetail.FailureString(),
 			paymentHash, paymentID)
 
 		return linkError
@@ -978,10 +977,12 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error {
 		// Check if the node is set to reject all onward HTLCs and also make
 		// sure that HTLC is not from the source node.
 		if s.cfg.RejectHTLC && packet.incomingChanID != hop.Source {
-			failure := &lnwire.FailChannelDisabled{}
+			failure := NewDetailedLinkError(
-			addErr := fmt.Errorf("unable to forward any htlcs")
+				&lnwire.FailChannelDisabled{},
+				OutgoingFailureForwardsDisabled,
+			)
 
-			return s.failAddPacket(packet, failure, addErr)
+			return s.failAddPacket(packet, failure)
 		}
 
 		if packet.incomingChanID == hop.Source {
@@ -1001,9 +1002,7 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error {
 			s.cfg.AllowCircularRoute, htlc.PaymentHash,
 		)
 		if linkErr != nil {
-			return s.failAddPacket(
+			return s.failAddPacket(packet, linkErr)
-				packet, linkErr.WireMessage(), linkErr,
-			)
 		}
 
 		s.indexMtx.RLock()
@@ -1011,14 +1010,17 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error {
 		if err != nil {
 			s.indexMtx.RUnlock()
 
+			log.Debugf("unable to find link with "+
+				"destination %v", packet.outgoingChanID)
+
 			// If packet was forwarded from another channel link
 			// than we should notify this link that some error
 			// occurred.
-			failure := &lnwire.FailUnknownNextPeer{}
+			linkError := NewLinkError(
-			addErr := fmt.Errorf("unable to find link with "+
+				&lnwire.FailUnknownNextPeer{},
-				"destination %v", packet.outgoingChanID)
+			)
 
-			return s.failAddPacket(packet, failure, addErr)
+			return s.failAddPacket(packet, linkError)
 		}
 		targetPeerKey := targetLink.Peer().PubKey()
 		interfaceLinks, _ := s.getLinks(targetPeerKey)
@@ -1041,7 +1043,7 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error {
 			if !link.EligibleToForward() {
 				failure = NewDetailedLinkError(
 					&lnwire.FailUnknownNextPeer{},
-					FailureDetailLinkNotEligible,
+					OutgoingFailureLinkNotEligible,
 				)
 			} else {
 				// We'll ensure that the HTLC satisfies the
@@ -1087,12 +1089,12 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error {
 					}))
 			}
 
-			addErr := fmt.Errorf("incoming HTLC(%x) violated "+
+			log.Tracef("incoming HTLC(%x) violated "+
 				"target outgoing link (id=%v) policy: %v",
 				htlc.PaymentHash[:], packet.outgoingChanID,
 				linkErr)
 
-			return s.failAddPacket(packet, linkErr.WireMessage(), addErr)
+			return s.failAddPacket(packet, linkErr)
 		}
 
 		// Send the packet to the destination channel link which
@@ -1217,20 +1219,18 @@ func checkCircularForward(incoming, outgoing lnwire.ShortChannelID,
 	// node, so we do not include a channel update.
 	return NewDetailedLinkError(
 		lnwire.NewTemporaryChannelFailure(nil),
-		FailureDetailCircularRoute,
+		OutgoingFailureCircularRoute,
 	)
 }
 
 // failAddPacket encrypts a fail packet back to an add packet's source.
 // The ciphertext will be derived from the failure message proivded by context.
 // This method returns the failErr if all other steps complete successfully.
-func (s *Switch) failAddPacket(packet *htlcPacket,
+func (s *Switch) failAddPacket(packet *htlcPacket, failure *LinkError) error {
-	failure lnwire.FailureMessage, failErr error) error {
-
 	// Encrypt the failure so that the sender will be able to read the error
 	// message. Since we failed this packet, we use EncryptFirstHop to
 	// obfuscate the failure for their eyes only.
-	reason, err := packet.obfuscator.EncryptFirstHop(failure)
+	reason, err := packet.obfuscator.EncryptFirstHop(failure.WireMessage())
 	if err != nil {
 		err := fmt.Errorf("unable to obfuscate "+
 			"error: %v", err)
@@ -1238,13 +1238,14 @@ func (s *Switch) failAddPacket(packet *htlcPacket,
 		return err
 	}
 
-	log.Error(failErr)
+	log.Error(failure.Error())
 
 	failPkt := &htlcPacket{
 		sourceRef:      packet.sourceRef,
 		incomingChanID: packet.incomingChanID,
 		incomingHTLCID: packet.incomingHTLCID,
 		circuit:        packet.circuit,
+		linkFailure:    failure,
 		htlc: &lnwire.UpdateFailHTLC{
 			Reason: reason,
 		},
@@ -1260,7 +1261,7 @@ func (s *Switch) failAddPacket(packet *htlcPacket,
 		return err
 	}
 
-	return failErr
+	return failure
 }
 
 // closeCircuit accepts a settle or fail htlc and the associated htlc packet and
@@ -1868,8 +1869,11 @@ func (s *Switch) reforwardSettleFails(fwdPkgs []*channeldb.FwdPkg) {
 			// commitment state, so we'll forward this to the switch so the
 			// backwards undo can continue.
 			case lnwallet.Fail:
-				// Fetch the reason the HTLC was canceled so we can
+				// Fetch the reason the HTLC was canceled so
-				// continue to propagate it.
+				// we can continue to propagate it. This
+				// failure originated from another node, so
+				// the linkFailure field is not set on this
+				// packet.
 				failPacket := &htlcPacket{
 					outgoingChanID: fwdPkg.Source,
 					outgoingHTLCID: pd.ParentIndex,

htlcswitch/switch_test.go

@@ -211,10 +211,13 @@ func TestSwitchSendPending(t *testing.T) {
 	// Send the ADD packet, this should not be forwarded out to the link
 	// since there are no eligible links.
 	err = s.forward(packet)
-	expErr := fmt.Sprintf("unable to find link with destination %v",
+	linkErr, ok := err.(*LinkError)
-		aliceChanID)
+	if !ok {
-	if err != nil && err.Error() != expErr {
+		t.Fatalf("expected link error, got: %T", err)
-		t.Fatalf("expected forward failure: %v", err)
+	}
+	if linkErr.WireMessage().Code() != lnwire.CodeUnknownNextPeer {
+		t.Fatalf("expected fail unknown next peer, got: %T",
+			linkErr.WireMessage().Code())
 	}
 
 	// No message should be sent, since the packet was failed.
@@ -1070,9 +1073,13 @@ func TestSwitchForwardFailAfterHalfAdd(t *testing.T) {
 	// Resend the failed htlc, it should be returned to alice since the
 	// switch will detect that it has been half added previously.
 	err = s2.forward(ogPacket)
-	if err != ErrIncompleteForward {
+	linkErr, ok := err.(*LinkError)
-		t.Fatal("unexpected error when reforwarding a "+
+	if !ok {
-			"failed packet", err)
+		t.Fatalf("expected link error, got: %T", err)
+	}
+	if linkErr.FailureDetail != OutgoingFailureIncompleteForward {
+		t.Fatalf("expected incomplete forward, got: %v",
+			linkErr.FailureDetail)
 	}
 
 	// After detecting an incomplete forward, the fail packet should have
@@ -1348,7 +1355,7 @@ func TestCircularForwards(t *testing.T) {
 			allowCircularPayment: false,
 			expectedErr: NewDetailedLinkError(
 				lnwire.NewTemporaryChannelFailure(nil),
-				FailureDetailCircularRoute,
+				OutgoingFailureCircularRoute,
 			),
 		},
 	}
@@ -1465,7 +1472,7 @@ func TestCheckCircularForward(t *testing.T) {
 			outgoingLink:  lnwire.NewShortChanIDFromInt(123),
 			expectedErr: NewDetailedLinkError(
 				lnwire.NewTemporaryChannelFailure(nil),
-				FailureDetailCircularRoute,
+				OutgoingFailureCircularRoute,
 			),
 		},
 	}
@@ -1527,7 +1534,7 @@ func TestSkipIneligibleLinksMultiHopForward(t *testing.T) {
 			eligible1: true,
 			failure1: NewDetailedLinkError(
 				lnwire.NewTemporaryChannelFailure(nil),
-				FailureDetailInsufficientBalance,
+				OutgoingFailureInsufficientBalance,
 			),
 			eligible2: true,
 			failure2: NewLinkError(

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
@@ -607,7 +565,7 @@ func (i *InvoiceRegistry) startHtlcTimer(hash lntypes.Hash,
 // 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, result ResolutionResult) error {
+	key channeldb.CircuitKey, result FailResolutionResult) error {
 
 	i.Lock()
 	defer i.Unlock()
@@ -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.
+			// Assign resolution to outer scope variable.
-			result = res
+			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.
+	switch res := resolution.(type) {
-	invoiceHtlc, ok := invoice.Htlcs[ctx.circuitKey]
+	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.
+		ctx.log(fmt.Sprintf("failure resolution result "+
-	if !ok {
+			"outcome: %v, at accept height: %v",
-		return NewFailureResolution(
+			res.Outcome, res.AcceptHeight))
-			ctx.circuitKey, ctx.currentHeight, result,
-		), nil
-	}
 
-	// Determine accepted height of this htlc. If the htlc reached the
+		return res, nil
-	// 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)
 
-	switch invoiceHtlc.State {
+	// If the htlc was settled, we will settle any previously accepted
-	case channeldb.HtlcStateCanceled:
+	// htlcs and notify our peer to settle them.
-		return NewFailureResolution(
+	case *HtlcSettleResolution:
-			ctx.circuitKey, acceptHeight, result,
+		ctx.log(fmt.Sprintf("settle resolution result "+
-		), nil
+			"outcome: %v, at accept height: %v",
+			res.Outcome, res.AcceptHeight))
 
-	case channeldb.HtlcStateSettled:
+		// Also settle any previously accepted htlcs. If a htlc is
-		// Also settle any previously accepted htlcs. The invoice state
+		// marked as settled, we should follow now and settle the htlc
-		// is leading. If an htlc is marked as settled, we should follow
+		// with our peer.
-		// 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(
+			htlcSettleResolution := NewSettleResolution(
-				invoice.Terms.PaymentPreimage, key,
+				res.Preimage, key,
-				acceptHeight, result,
+				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:
+	// If we accepted the htlc, subscribe to the hodl invoice and return
-		var resolution acceptResolution
+	// 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
+			res.acceptTime = invoiceHtlc.AcceptTime
-			resolution.autoRelease = true
+			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 {
+	failResolution, ok := resolution.(*HtlcFailResolution)
-		t.Fatal("expected cancel resolution")
+	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,
+		testInvoicePaymentHash, amtPaid, testHtlcExpiry,
-		getCircuitKey(0), hodlChan, testPayload,
+		testCurrentHeight, getCircuitKey(0), hodlChan,
+		testPayload,
 	)
 	if err != nil {
 		t.Fatal(err)
 	}
-	if resolution.Outcome != ResultSettled {
+	settleResolution, ok := resolution.(*HtlcSettleResolution)
-		t.Fatalf("expected settled, got: %v", resolution.Outcome)
+	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 {
+	settleResolution, ok = resolution.(*HtlcSettleResolution)
-		t.Fatal("expected settle resolution")
+	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 {
+	settleResolution, ok = resolution.(*HtlcSettleResolution)
-		t.Fatal("expected settle resolution")
+	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 {
+	failResolution, ok = resolution.(*HtlcFailResolution)
-		t.Fatal("expected cancel resolution")
+	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")
 	}
-
+	failResolution, ok := resolution.(*HtlcFailResolution)
-	if resolution.Preimage != nil {
+	if !ok {
-		t.Fatal("expected cancel htlc resolution")
+		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 {
+	if failResolution.Outcome != ResultInvoiceAlreadyCanceled {
-		t.Fatalf("expected invoice already canceled, got: %v",
+		t.Fatalf("expected expiry too soon, got: %v",
-			resolution.Outcome)
+			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 {
+	failResolution, ok := resolution.(*HtlcFailResolution)
-		t.Fatalf("expected htlc to be canceled")
+	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 {
+	_, ok := htlcResolution.(*HtlcFailResolution)
-		t.Fatal("expected cancel htlc resolution")
+	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 {
+	failResolution, ok := resolution.(*HtlcFailResolution)
-		t.Fatalf("expected htlc to be canceled")
+	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)
 	}
-
+	failResolution, ok := resolution.(*HtlcFailResolution)
-	// Expect a cancel resolution with the correct outcome.
+	if !ok {
-	if resolution.Preimage != nil {
+		t.Fatalf("expected fail resolution, got: %T",
-		t.Fatal("expected cancel resolution")
+			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 {
+	settleResolution, ok := resolution.(*HtlcSettleResolution)
-		t.Fatal("expected valid settle event")
+	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 {
+	failResolution, ok := htlcResolution.(*HtlcFailResolution)
-		t.Fatal("expected cancel resolution")
+	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 {
+	settleResolution, ok := resolution.(*HtlcSettleResolution)
-		t.Fatal("expected a settle resolution")
+	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,125 @@
+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 FailResolutionResult
+}
+
+// NewFailResolution returns a htlc failure resolution.
+func NewFailResolution(key channeldb.CircuitKey,
+	acceptHeight int32, outcome FailResolutionResult) *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 SettleResolutionResult
+}
+
+// NewSettleResolution returns a htlc resolution which is associated with a
+// settle.
+func NewSettleResolution(preimage lntypes.Preimage,
+	key channeldb.CircuitKey, acceptHeight int32,
+	outcome SettleResolutionResult) *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 acceptResolutionResult
+}
+
+// newAcceptResolution returns a htlc resolution which is associated with a
+// htlc accept.
+func newAcceptResolution(key channeldb.CircuitKey,
+	outcome acceptResolutionResult) *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/resolution_result.go

@@ -0,0 +1,202 @@
+package invoices
+
+// acceptResolutionResult provides metadata which about a htlc that was
+// accepted by the registry.
+type acceptResolutionResult uint8
+
+const (
+	resultInvalidAccept acceptResolutionResult = iota
+
+	// resultReplayToAccepted is returned when we replay an accepted
+	// invoice.
+	resultReplayToAccepted
+
+	// resultDuplicateToAccepted is returned when we accept a duplicate
+	// htlc.
+	resultDuplicateToAccepted
+
+	// resultAccepted is returned when we accept a hodl invoice.
+	resultAccepted
+
+	// resultPartialAccepted is returned when we have partially received
+	// payment.
+	resultPartialAccepted
+)
+
+// String returns a string representation of the result.
+func (a acceptResolutionResult) String() string {
+	switch a {
+	case resultInvalidAccept:
+		return "invalid accept result"
+
+	case resultReplayToAccepted:
+		return "replayed htlc to accepted invoice"
+
+	case resultDuplicateToAccepted:
+		return "accepting duplicate payment to accepted invoice"
+
+	case resultAccepted:
+		return "accepted"
+
+	case resultPartialAccepted:
+		return "partial payment accepted"
+
+	default:
+		return "unknown accept resolution result"
+	}
+}
+
+// FailResolutionResult provides metadata about a htlc that was failed by
+// the registry. It can be used to take custom actions on resolution of the
+// htlc.
+type FailResolutionResult uint8
+
+const (
+	resultInvalidFailure FailResolutionResult = iota
+
+	// ResultReplayToCanceled is returned when we replay a canceled invoice.
+	ResultReplayToCanceled
+
+	// ResultInvoiceAlreadyCanceled is returned when trying to pay an
+	// invoice that is already canceled.
+	ResultInvoiceAlreadyCanceled
+
+	// ResultAmountTooLow is returned when an invoice is underpaid.
+	ResultAmountTooLow
+
+	// ResultExpiryTooSoon is returned when we do not accept an invoice
+	// payment because it expires too soon.
+	ResultExpiryTooSoon
+
+	// ResultCanceled is returned when we cancel an invoice and its
+	// associated htlcs.
+	ResultCanceled
+
+	// ResultInvoiceNotOpen is returned when a mpp invoice is not open.
+	ResultInvoiceNotOpen
+
+	// 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
+
+	// ResultHtlcSetTotalMismatch is returned when the amount paid by a
+	// htlc does not match its set total.
+	ResultHtlcSetTotalMismatch
+
+	// ResultHtlcSetTotalTooLow is returned when a mpp set total is too low
+	// for an invoice.
+	ResultHtlcSetTotalTooLow
+
+	// ResultHtlcSetOverpayment is returned when a mpp set is overpaid.
+	ResultHtlcSetOverpayment
+
+	// ResultInvoiceNotFound is returned when an attempt is made to pay an
+	// invoice that is unknown to us.
+	ResultInvoiceNotFound
+
+	// ResultKeySendError is returned when we receive invalid keysend
+	// parameters.
+	ResultKeySendError
+
+	// ResultMppInProgress is returned when we are busy receiving a mpp
+	// payment.
+	ResultMppInProgress
+)
+
+// FailureString returns a string representation of the result.
+//
+// Note: it is part of the FailureDetail interface.
+func (f FailResolutionResult) FailureString() string {
+	switch f {
+	case resultInvalidFailure:
+		return "invalid failure result"
+
+	case ResultReplayToCanceled:
+		return "replayed htlc to canceled invoice"
+
+	case ResultInvoiceAlreadyCanceled:
+		return "invoice already canceled"
+
+	case ResultAmountTooLow:
+		return "amount too low"
+
+	case ResultExpiryTooSoon:
+		return "expiry too soon"
+
+	case ResultCanceled:
+		return "canceled"
+
+	case ResultInvoiceNotOpen:
+		return "invoice no longer open"
+
+	case ResultMppTimeout:
+		return "mpp timeout"
+
+	case ResultAddressMismatch:
+		return "payment address mismatch"
+
+	case ResultHtlcSetTotalMismatch:
+		return "htlc total amt doesn't match set total"
+
+	case ResultHtlcSetTotalTooLow:
+		return "set total too low for invoice"
+
+	case ResultHtlcSetOverpayment:
+		return "mpp is overpaying set total"
+
+	case ResultInvoiceNotFound:
+		return "invoice not found"
+
+	case ResultKeySendError:
+		return "invalid keysend parameters"
+
+	case ResultMppInProgress:
+		return "mpp reception in progress"
+
+	default:
+		return "unknown failure resolution result"
+	}
+}
+
+// SettleResolutionResult provides metadata which about a htlc that was failed
+// by the registry. It can be used to take custom actions on resolution of the
+// htlc.
+type SettleResolutionResult uint8
+
+const (
+	resultInvalidSettle SettleResolutionResult = iota
+
+	// ResultSettled is returned when we settle an invoice.
+	ResultSettled
+
+	// ResultReplayToSettled is returned when we replay a settled invoice.
+	ResultReplayToSettled
+
+	// ResultDuplicateToSettled is returned when we settle an invoice which
+	// has already been settled at least once.
+	ResultDuplicateToSettled
+)
+
+// String returns a string representation of the result.
+func (s SettleResolutionResult) String() string {
+	switch s {
+	case resultInvalidSettle:
+		return "invalid settle result"
+
+	case ResultSettled:
+		return "settled"
+
+	case ResultReplayToSettled:
+		return "replayed htlc to settled invoice"
+
+	case ResultDuplicateToSettled:
+		return "accepting duplicate payment to settled invoice"
+
+	default:
+		return "unknown settle resolution result"
+	}
+}

invoices/update.go

@@ -9,164 +9,6 @@ import (
 	"github.com/lightningnetwork/lnd/record"
 )
 
-// ResolutionResult provides metadata which about an invoice update which can
-// be used to take custom actions on resolution of the htlc. Only results which
-// are actionable by the link are exported.
-type ResolutionResult uint8
-
-const (
-	resultInvalid ResolutionResult = iota
-
-	// ResultReplayToCanceled is returned when we replay a canceled invoice.
-	ResultReplayToCanceled
-
-	// ResultReplayToAccepted is returned when we replay an accepted invoice.
-	ResultReplayToAccepted
-
-	// ResultReplayToSettled is returned when we replay a settled invoice.
-	ResultReplayToSettled
-
-	// ResultInvoiceAlreadyCanceled is returned when trying to pay an invoice
-	// that is already canceled.
-	ResultInvoiceAlreadyCanceled
-
-	// ResultAmountTooLow is returned when an invoice is underpaid.
-	ResultAmountTooLow
-
-	// ResultExpiryTooSoon is returned when we do not accept an invoice payment
-	// because it expires too soon.
-	ResultExpiryTooSoon
-
-	// ResultDuplicateToAccepted is returned when we accept a duplicate htlc.
-	ResultDuplicateToAccepted
-
-	// ResultDuplicateToSettled is returned when we settle an invoice which has
-	// already been settled at least once.
-	ResultDuplicateToSettled
-
-	// ResultAccepted is returned when we accept a hodl invoice.
-	ResultAccepted
-
-	// 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
-
-	// ResultPartialAccepted is returned when we have partially received
-	// payment.
-	ResultPartialAccepted
-
-	// 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
-
-	// ResultHtlcSetTotalMismatch is returned when the amount paid by a htlc
-	// does not match its set total.
-	ResultHtlcSetTotalMismatch
-
-	// ResultHtlcSetTotalTooLow is returned when a mpp set total is too low for
-	// an invoice.
-	ResultHtlcSetTotalTooLow
-
-	// ResultHtlcSetOverpayment is returned when a mpp set is overpaid.
-	ResultHtlcSetOverpayment
-
-	// ResultInvoiceNotFound is returned when an attempt is made to pay an
-	// invoice that is unknown to us.
-	ResultInvoiceNotFound
-
-	// ResultKeySendError is returned when we receive invalid keysend
-	// parameters.
-	ResultKeySendError
-)
-
-// String returns a human-readable representation of the invoice update result.
-func (u ResolutionResult) String() string {
-	switch u {
-
-	case resultInvalid:
-		return "invalid"
-
-	case ResultReplayToCanceled:
-		return "replayed htlc to canceled invoice"
-
-	case ResultReplayToAccepted:
-		return "replayed htlc to accepted invoice"
-
-	case ResultReplayToSettled:
-		return "replayed htlc to settled invoice"
-
-	case ResultInvoiceAlreadyCanceled:
-		return "invoice already canceled"
-
-	case ResultAmountTooLow:
-		return "amount too low"
-
-	case ResultExpiryTooSoon:
-		return "expiry too soon"
-
-	case ResultDuplicateToAccepted:
-		return "accepting duplicate payment to accepted invoice"
-
-	case ResultDuplicateToSettled:
-		return "accepting duplicate payment to settled invoice"
-
-	case ResultAccepted:
-		return "accepted"
-
-	case ResultSettled:
-		return "settled"
-
-	case ResultCanceled:
-		return "canceled"
-
-	case ResultInvoiceNotOpen:
-		return "invoice no longer open"
-
-	case ResultPartialAccepted:
-		return "partial payment accepted"
-
-	case ResultMppInProgress:
-		return "mpp reception in progress"
-
-	case ResultMppTimeout:
-		return "mpp timeout"
-
-	case ResultAddressMismatch:
-		return "payment address mismatch"
-
-	case ResultHtlcSetTotalMismatch:
-		return "htlc total amt doesn't match set total"
-
-	case ResultHtlcSetTotalTooLow:
-		return "set total too low for invoice"
-
-	case ResultHtlcSetOverpayment:
-		return "mpp is overpaying set total"
-
-	case ResultKeySendError:
-		return "invalid keysend parameters"
-
-	case ResultInvoiceNotFound:
-		return "invoice not found"
-
-	default:
-		return "unknown"
-	}
-}
-
 // invoiceUpdateCtx is an object that describes the context for the invoice
 // update to be carried out.
 type invoiceUpdateCtx struct {
@@ -186,26 +28,55 @@ 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 fail resolution
+// result provided.
+func (i invoiceUpdateCtx) failRes(outcome FailResolutionResult) *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
+// the settle resolution result provided.
+func (i invoiceUpdateCtx) settleRes(preimage lntypes.Preimage,
+	outcome SettleResolutionResult) *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 accept resolution
+// result provided.
+func (i invoiceUpdateCtx) acceptRes(outcome acceptResolutionResult) *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 +89,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) (
+func updateMpp(ctx *invoiceUpdateCtx,
-	*channeldb.InvoiceUpdateDesc, ResolutionResult, error) {
+	inv *channeldb.Invoice) (*channeldb.InvoiceUpdateDesc,
+	HtlcResolution, error) {
 
 	// Start building the accept descriptor.
 	acceptDesc := &channeldb.HtlcAcceptDesc{
@@ -235,23 +107,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 +137,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 +148,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 +172,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 +182,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 +190,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) (
+func updateLegacy(ctx *invoiceUpdateCtx,
-	*channeldb.InvoiceUpdateDesc, ResolutionResult, error) {
+	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 +211,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 +224,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 +255,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 +270,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 +279,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
 }