htlcswitch: add linkError field to htlcpacket

This commit adds a linkError field to track the value of failures
which occur at our node. This field is set when local payments or
multi hop htlcs fail in the switch or on our outgoing link. This
addition is required for the addition of a htlc notifier which will
notify these failures in handleDownstreamPacket.

The passing of link error to failAddPacket removes the need for an
additional error field, because the link error's failure detail will
contain any additional metadata. In the places where the failure detail
does not cover all the metadata that was previously supplied by addr
err, the error is logged before calling failAddPacket so that this
change does not reduce the amount of information we log.

htlcswitch/failure.go

@@ -76,7 +76,7 @@ func (l *LinkError) Error() string {
 		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

@@ -42,6 +42,18 @@ const (
 	// to forward a htlc through our node which arrives and leaves on the
 	// same channel.
 	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
 )
 
 // FailureString returns the string representation of a failure detail.
@@ -70,6 +82,15 @@ func (fd OutgoingFailure) FailureString() string {
 	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/link.go

@@ -1314,6 +1314,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(
@@ -1322,28 +1326,43 @@ func (l *channelLink) handleDownStreamPkt(pkt *htlcPacket, isReProcess bool) {
 					},
 				)
 
-				// Encrypt the error back to the source unless
-				// the payment was generated locally.
+				// If the payment was locally initiated (which
+				// 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,
@@ -1351,6 +1370,7 @@ func (l *channelLink) handleDownStreamPkt(pkt *htlcPacket, isReProcess bool) {
 					sourceRef:      pkt.sourceRef,
 					hasSource:      true,
 					localFailure:   localFailure,
+					linkFailure:    linkError,
 					htlc: &lnwire.UpdateFailHTLC{
 						Reason: reason,
 					},
@@ -2461,7 +2481,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,

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)
 		}
 	}
 
@@ -979,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{}
-			addErr := fmt.Errorf("unable to forward any htlcs")
+			failure := NewDetailedLinkError(
+				&lnwire.FailChannelDisabled{},
+				OutgoingFailureForwardsDisabled,
+			)
 
-			return s.failAddPacket(packet, failure, addErr)
+			return s.failAddPacket(packet, failure)
 		}
 
 		if packet.incomingChanID == hop.Source {
@@ -1002,9 +1002,7 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error {
 			s.cfg.AllowCircularRoute, htlc.PaymentHash,
 		)
 		if linkErr != nil {
-			return s.failAddPacket(
-				packet, linkErr.WireMessage(), linkErr,
-			)
+			return s.failAddPacket(packet, linkErr)
 		}
 
 		s.indexMtx.RLock()
@@ -1012,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{}
-			addErr := fmt.Errorf("unable to find link with "+
-				"destination %v", packet.outgoingChanID)
+			linkError := NewLinkError(
+				&lnwire.FailUnknownNextPeer{},
+			)
 
-			return s.failAddPacket(packet, failure, addErr)
+			return s.failAddPacket(packet, linkError)
 		}
 		targetPeerKey := targetLink.Peer().PubKey()
 		interfaceLinks, _ := s.getLinks(targetPeerKey)
@@ -1088,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
@@ -1225,13 +1226,11 @@ func checkCircularForward(incoming, outgoing lnwire.ShortChannelID,
 // 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,
-	failure lnwire.FailureMessage, failErr error) error {
-
+func (s *Switch) failAddPacket(packet *htlcPacket, failure *LinkError) 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)
@@ -1239,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,
 		},
@@ -1261,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
@@ -1869,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
-				// continue to propagate it.
+				// Fetch the reason the HTLC was canceled so
+				// 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",
-		aliceChanID)
-	if err != nil && err.Error() != expErr {
-		t.Fatalf("expected forward failure: %v", err)
+	linkErr, ok := err.(*LinkError)
+	if !ok {
+		t.Fatalf("expected link error, got: %T", 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 {
-		t.Fatal("unexpected error when reforwarding a "+
-			"failed packet", err)
+	linkErr, ok := err.(*LinkError)
+	if !ok {
+		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