htlcswitch: use LinkError for internal errors

Update the ChannelLink interface to specifically
return the LinkError struct. This error implements
the ClearTextError interface, so will be picked
up as a routing realted error by the router.

With LinkErrors implemented, the switch now
returns a LinkError for all failures on our
incoming/outgoing link and ForwardingError when
the failure occurs down the line.

htlcswitch/failure.go

@@ -33,6 +33,9 @@ type LinkError struct {
 	// know the failure type for failures which occur at our own
 	// node.
 	msg lnwire.FailureMessage
+
+	// FailureDetail enriches the wire error with additional information.
+	FailureDetail
 }
 
 // NewLinkError returns a LinkError with the failure message provided.
@@ -42,6 +45,17 @@ func NewLinkError(msg lnwire.FailureMessage) *LinkError {
 	return &LinkError{msg: msg}
 }
 
+// NewDetailedLinkError returns a link error that enriches a wire message with
+// a failure detail.
+func NewDetailedLinkError(msg lnwire.FailureMessage,
+	detail FailureDetail) *LinkError {
+
+	return &LinkError{
+		msg:           msg,
+		FailureDetail: detail,
+	}
+}
+
 // WireMessage extracts a valid wire failure message from an internal
 // error which may contain additional metadata (which should not be
 // exposed to the network). This value should never be nil for LinkErrors,
@@ -56,7 +70,13 @@ func (l *LinkError) WireMessage() lnwire.FailureMessage {
 //
 // Note this is part of the ClearTextError interface.
 func (l *LinkError) Error() string {
-	return l.msg.Error()
+	// If the link error has no failure detail, return the wire message's
+	// error.
+	if l.FailureDetail == FailureDetailNone {
+		return l.msg.Error()
+	}
+
+	return fmt.Sprintf("%v: %v", l.msg.Error(), l.FailureDetail)
 }
 
 // ForwardingError wraps an lnwire.FailureMessage in a struct that also

htlcswitch/failure_detail.go

@@ -0,0 +1,58 @@
+package htlcswitch
+
+// FailureDetail is an enum which is used to enrich failures with
+// additional information.
+type FailureDetail int
+
+const (
+	// FailureDetailNone is returned when the wire message contains
+	// sufficient information.
+	FailureDetailNone = iota
+
+	// FailureDetailOnionDecode indicates that we could not decode an
+	// onion error.
+	FailureDetailOnionDecode
+
+	// FailureDetailLinkNotEligible indicates that a routing attempt was
+	// made over a link that is not eligible for routing.
+	FailureDetailLinkNotEligible
+
+	// FailureDetailOnChainTimeout indicates that a payment had to be timed
+	// out on chain before it got past the first hop by us or the remote
+	// party.
+	FailureDetailOnChainTimeout
+
+	// FailureDetailHTLCExceedsMax is returned when a htlc exceeds our
+	// policy's maximum htlc amount.
+	FailureDetailHTLCExceedsMax
+
+	// FailureDetailInsufficientBalance is returned when we cannot route a
+	// htlc due to insufficient outgoing capacity.
+	FailureDetailInsufficientBalance
+)
+
+// String returns the string representation of a failure detail.
+func (fd FailureDetail) String() string {
+	switch fd {
+	case FailureDetailNone:
+		return "no failure detail"
+
+	case FailureDetailOnionDecode:
+		return "could not decode onion"
+
+	case FailureDetailLinkNotEligible:
+		return "link not eligible"
+
+	case FailureDetailOnChainTimeout:
+		return "payment was resolved on-chain, then canceled back"
+
+	case FailureDetailHTLCExceedsMax:
+		return "htlc exceeds maximum policy amount"
+
+	case FailureDetailInsufficientBalance:
+		return "insufficient bandwidth to route htlc"
+
+	default:
+		return "unknown failure detail"
+	}
+}

htlcswitch/interfaces.go

@@ -102,20 +102,21 @@ type ChannelLink interface {
 
 	// CheckHtlcForward should return a nil error if the passed HTLC details
 	// satisfy the current forwarding policy fo the target link. Otherwise,
-	// a valid protocol failure message should be returned in order to
+	// a LinkError with a valid protocol failure message should be returned
-	// signal to the source of the HTLC, the policy consistency issue.
+	// in order to signal to the source of the HTLC, the policy consistency
+	// issue.
 	CheckHtlcForward(payHash [32]byte, incomingAmt lnwire.MilliSatoshi,
 		amtToForward lnwire.MilliSatoshi,
 		incomingTimeout, outgoingTimeout uint32,
-		heightNow uint32) lnwire.FailureMessage
+		heightNow uint32) *LinkError
 
 	// CheckHtlcTransit should return a nil error if the passed HTLC details
-	// satisfy the current channel policy.  Otherwise, a valid protocol
+	// satisfy the current channel policy.  Otherwise, a LinkError with a
-	// failure message should be returned in order to signal the violation.
+	// valid protocol failure message should be returned in order to signal
-	// This call is intended to be used for locally initiated payments for
+	// the violation. This call is intended to be used for locally initiated
-	// which there is no corresponding incoming htlc.
+	// payments for which there is no corresponding incoming htlc.
 	CheckHtlcTransit(payHash [32]byte, amt lnwire.MilliSatoshi,
-		timeout uint32, heightNow uint32) lnwire.FailureMessage
+		timeout uint32, heightNow uint32) *LinkError
 
 	// Bandwidth returns the amount of milli-satoshis which current link
 	// might pass through channel link. The value returned from this method

htlcswitch/link.go

@@ -2135,16 +2135,17 @@ func (l *channelLink) UpdateForwardingPolicy(newPolicy ForwardingPolicy) {
 	l.cfg.FwrdingPolicy = newPolicy
 }
 
-// CheckHtlcForward should return a nil error if the passed HTLC details satisfy
+// CheckHtlcForward should return a nil error if the passed HTLC details
-// the current forwarding policy fo the target link.  Otherwise, a valid
+// satisfy the current forwarding policy fo the target link. Otherwise,
-// protocol failure message should be returned in order to signal to the source
+// a LinkError with a valid protocol failure message should be returned
-// of the HTLC, the policy consistency issue.
+// in order to signal to the source of the HTLC, the policy consistency
+// issue.
 //
 // NOTE: Part of the ChannelLink interface.
 func (l *channelLink) CheckHtlcForward(payHash [32]byte,
 	incomingHtlcAmt, amtToForward lnwire.MilliSatoshi,
 	incomingTimeout, outgoingTimeout uint32,
-	heightNow uint32) lnwire.FailureMessage {
+	heightNow uint32) *LinkError {
 
 	l.RLock()
 	policy := l.cfg.FwrdingPolicy
@@ -2176,14 +2177,14 @@ func (l *channelLink) CheckHtlcForward(payHash [32]byte,
 
 		// As part of the returned error, we'll send our latest routing
 		// policy so the sending node obtains the most up to date data.
-
+		failure := l.createFailureWithUpdate(
-		return l.createFailureWithUpdate(
 			func(upd *lnwire.ChannelUpdate) lnwire.FailureMessage {
 				return lnwire.NewFeeInsufficient(
 					amtToForward, *upd,
 				)
 			},
 		)
+		return NewLinkError(failure)
 	}
 
 	// Finally, we'll ensure that the time-lock on the outgoing HTLC meets
@@ -2198,26 +2199,27 @@ func (l *channelLink) CheckHtlcForward(payHash [32]byte,
 
 		// Grab the latest routing policy so the sending node is up to
 		// date with our current policy.
-		return l.createFailureWithUpdate(
+		failure := l.createFailureWithUpdate(
 			func(upd *lnwire.ChannelUpdate) lnwire.FailureMessage {
 				return lnwire.NewIncorrectCltvExpiry(
 					incomingTimeout, *upd,
 				)
 			},
 		)
+		return NewLinkError(failure)
 	}
 
 	return nil
 }
 
-// CheckHtlcTransit should return a nil error if the passed HTLC details satisfy the
+// CheckHtlcTransit should return a nil error if the passed HTLC details
-// current channel policy.  Otherwise, a valid protocol failure message should
+// satisfy the current channel policy.  Otherwise, a LinkError with a
-// be returned in order to signal the violation. This call is intended to be
+// valid protocol failure message should be returned in order to signal
-// used for locally initiated payments for which there is no corresponding
+// the violation. This call is intended to be used for locally initiated
-// incoming htlc.
+// payments for which there is no corresponding incoming htlc.
 func (l *channelLink) CheckHtlcTransit(payHash [32]byte,
 	amt lnwire.MilliSatoshi, timeout uint32,
-	heightNow uint32) lnwire.FailureMessage {
+	heightNow uint32) *LinkError {
 
 	l.RLock()
 	policy := l.cfg.FwrdingPolicy
@@ -2232,7 +2234,7 @@ func (l *channelLink) CheckHtlcTransit(payHash [32]byte,
 // the channel's amount and time lock constraints.
 func (l *channelLink) canSendHtlc(policy ForwardingPolicy,
 	payHash [32]byte, amt lnwire.MilliSatoshi, timeout uint32,
-	heightNow uint32) lnwire.FailureMessage {
+	heightNow uint32) *LinkError {
 
 	// As our first sanity check, we'll ensure that the passed HTLC isn't
 	// too small for the next hop. If so, then we'll cancel the HTLC
@@ -2244,13 +2246,14 @@ func (l *channelLink) canSendHtlc(policy ForwardingPolicy,
 
 		// As part of the returned error, we'll send our latest routing
 		// policy so the sending node obtains the most up to date data.
-		return l.createFailureWithUpdate(
+		failure := l.createFailureWithUpdate(
 			func(upd *lnwire.ChannelUpdate) lnwire.FailureMessage {
 				return lnwire.NewAmountBelowMinimum(
 					amt, *upd,
 				)
 			},
 		)
+		return NewLinkError(failure)
 	}
 
 	// Next, ensure that the passed HTLC isn't too large. If so, we'll
@@ -2261,11 +2264,12 @@ func (l *channelLink) canSendHtlc(policy ForwardingPolicy,
 
 		// As part of the returned error, we'll send our latest routing
 		// policy so the sending node obtains the most up-to-date data.
-		return l.createFailureWithUpdate(
+		failure := l.createFailureWithUpdate(
 			func(upd *lnwire.ChannelUpdate) lnwire.FailureMessage {
 				return lnwire.NewTemporaryChannelFailure(upd)
 			},
 		)
+		return NewDetailedLinkError(failure, FailureDetailHTLCExceedsMax)
 	}
 
 	// We want to avoid offering an HTLC which will expire in the near
@@ -2275,12 +2279,12 @@ func (l *channelLink) canSendHtlc(policy ForwardingPolicy,
 		l.log.Errorf("htlc(%x) has an expiry that's too soon: "+
 			"outgoing_expiry=%v, best_height=%v", payHash[:],
 			timeout, heightNow)
-
+		failure := l.createFailureWithUpdate(
-		return l.createFailureWithUpdate(
 			func(upd *lnwire.ChannelUpdate) lnwire.FailureMessage {
 				return lnwire.NewExpiryTooSoon(*upd)
 			},
 		)
+		return NewLinkError(failure)
 	}
 
 	// Check absolute max delta.
@@ -2289,16 +2293,19 @@ func (l *channelLink) canSendHtlc(policy ForwardingPolicy,
 			"the future: got %v, but maximum is %v", payHash[:],
 			timeout-heightNow, l.cfg.MaxOutgoingCltvExpiry)
 
-		return &lnwire.FailExpiryTooFar{}
+		return NewLinkError(&lnwire.FailExpiryTooFar{})
 	}
 
 	// Check to see if there is enough balance in this channel.
 	if amt > l.Bandwidth() {
-		return l.createFailureWithUpdate(
+		failure := l.createFailureWithUpdate(
 			func(upd *lnwire.ChannelUpdate) lnwire.FailureMessage {
 				return lnwire.NewTemporaryChannelFailure(upd)
 			},
 		)
+		return NewDetailedLinkError(
+			failure, FailureDetailInsufficientBalance,
+		)
 	}
 
 	return nil

htlcswitch/link_test.go

@@ -5541,7 +5541,7 @@ func TestCheckHtlcForward(t *testing.T) {
 	t.Run("below minhtlc", func(t *testing.T) {
 		result := link.CheckHtlcForward(hash, 100, 50,
 			200, 150, 0)
-		if _, ok := result.(*lnwire.FailAmountBelowMinimum); !ok {
+		if _, ok := result.WireMessage().(*lnwire.FailAmountBelowMinimum); !ok {
 			t.Fatalf("expected FailAmountBelowMinimum failure code")
 		}
 	})
@@ -5549,7 +5549,7 @@ func TestCheckHtlcForward(t *testing.T) {
 	t.Run("above maxhtlc", func(t *testing.T) {
 		result := link.CheckHtlcForward(hash, 1500, 1200,
 			200, 150, 0)
-		if _, ok := result.(*lnwire.FailTemporaryChannelFailure); !ok {
+		if _, ok := result.WireMessage().(*lnwire.FailTemporaryChannelFailure); !ok {
 			t.Fatalf("expected FailTemporaryChannelFailure failure code")
 		}
 	})
@@ -5557,7 +5557,7 @@ func TestCheckHtlcForward(t *testing.T) {
 	t.Run("insufficient fee", func(t *testing.T) {
 		result := link.CheckHtlcForward(hash, 1005, 1000,
 			200, 150, 0)
-		if _, ok := result.(*lnwire.FailFeeInsufficient); !ok {
+		if _, ok := result.WireMessage().(*lnwire.FailFeeInsufficient); !ok {
 			t.Fatalf("expected FailFeeInsufficient failure code")
 		}
 	})
@@ -5565,7 +5565,7 @@ func TestCheckHtlcForward(t *testing.T) {
 	t.Run("expiry too soon", func(t *testing.T) {
 		result := link.CheckHtlcForward(hash, 1500, 1000,
 			200, 150, 190)
-		if _, ok := result.(*lnwire.FailExpiryTooSoon); !ok {
+		if _, ok := result.WireMessage().(*lnwire.FailExpiryTooSoon); !ok {
 			t.Fatalf("expected FailExpiryTooSoon failure code")
 		}
 	})
@@ -5573,7 +5573,7 @@ func TestCheckHtlcForward(t *testing.T) {
 	t.Run("incorrect cltv expiry", func(t *testing.T) {
 		result := link.CheckHtlcForward(hash, 1500, 1000,
 			200, 190, 0)
-		if _, ok := result.(*lnwire.FailIncorrectCltvExpiry); !ok {
+		if _, ok := result.WireMessage().(*lnwire.FailIncorrectCltvExpiry); !ok {
 			t.Fatalf("expected FailIncorrectCltvExpiry failure code")
 		}
 
@@ -5583,7 +5583,7 @@ func TestCheckHtlcForward(t *testing.T) {
 		// Check that expiry isn't too far in the future.
 		result := link.CheckHtlcForward(hash, 1500, 1000,
 			10200, 10100, 0)
-		if _, ok := result.(*lnwire.FailExpiryTooFar); !ok {
+		if _, ok := result.WireMessage().(*lnwire.FailExpiryTooFar); !ok {
 			t.Fatalf("expected FailExpiryTooFar failure code")
 		}
 	})

htlcswitch/mock.go

@@ -645,9 +645,9 @@ type mockChannelLink struct {
 
 	htlcID uint64
 
-	checkHtlcTransitResult lnwire.FailureMessage
+	checkHtlcTransitResult *LinkError
 
-	checkHtlcForwardResult lnwire.FailureMessage
+	checkHtlcForwardResult *LinkError
 }
 
 // completeCircuit is a helper method for adding the finalized payment circuit
@@ -707,14 +707,14 @@ func (f *mockChannelLink) HandleChannelUpdate(lnwire.Message) {
 func (f *mockChannelLink) UpdateForwardingPolicy(_ ForwardingPolicy) {
 }
 func (f *mockChannelLink) CheckHtlcForward([32]byte, lnwire.MilliSatoshi,
-	lnwire.MilliSatoshi, uint32, uint32, uint32) lnwire.FailureMessage {
+	lnwire.MilliSatoshi, uint32, uint32, uint32) *LinkError {
 
 	return f.checkHtlcForwardResult
 }
 
 func (f *mockChannelLink) CheckHtlcTransit(payHash [32]byte,
 	amt lnwire.MilliSatoshi, timeout uint32,
-	heightNow uint32) lnwire.FailureMessage {
+	heightNow uint32) *LinkError {
 
 	return f.checkHtlcTransitResult
 }

htlcswitch/switch.go

@@ -756,20 +756,19 @@ func (s *Switch) handleLocalDispatch(pkt *htlcPacket) error {
 		s.indexMtx.RUnlock()
 		if err != nil {
 			log.Errorf("Link %v not found", pkt.outgoingChanID)
-			return NewForwardingError(
+			return NewLinkError(&lnwire.FailUnknownNextPeer{})
-				&lnwire.FailUnknownNextPeer{}, 0, "",
-			)
 		}
 
 		if !link.EligibleToForward() {
-			err := fmt.Errorf("Link %v is not available to forward",
+			log.Errorf("Link %v is not available to forward",
 				pkt.outgoingChanID)
-			log.Error(err)
 
 			// The update does not need to be populated as the error
 			// will be returned back to the router.
-			htlcErr := lnwire.NewTemporaryChannelFailure(nil)
+			return NewDetailedLinkError(
-			return NewForwardingError(htlcErr, 0, err.Error())
+				lnwire.NewTemporaryChannelFailure(nil),
+				FailureDetailLinkNotEligible,
+			)
 		}
 
 		// Ensure that the htlc satisfies the outgoing channel policy.
@@ -782,8 +781,7 @@ func (s *Switch) handleLocalDispatch(pkt *htlcPacket) error {
 		if htlcErr != nil {
 			log.Errorf("Link %v policy for local forward not "+
 				"satisfied", pkt.outgoingChanID)
-
+			return htlcErr
-			return NewForwardingError(htlcErr, 0, "")
 		}
 
 		return link.HandleSwitchPacket(pkt)
@@ -907,35 +905,43 @@ func (s *Switch) parseFailedPayment(deobfuscator ErrorDecrypter,
 	// decrypt the error, simply decode it them report back to the
 	// user.
 	case unencrypted:
-		var userErr string
 		r := bytes.NewReader(htlc.Reason)
 		failureMsg, err := lnwire.DecodeFailure(r, 0)
 		if err != nil {
-			userErr = fmt.Sprintf("unable to decode onion "+
+			// If we could not decode the failure reason, return a link
-				"failure (hash=%v, pid=%d): %v",
+			// error indicating that we failed to decode the onion.
-				paymentHash, paymentID, err)
+			linkError := NewDetailedLinkError(
-			log.Error(userErr)
+				// As this didn't even clear the link, we don't
+				// need to apply an update here since it goes
+				// directly to the router.
+				lnwire.NewTemporaryChannelFailure(nil),
+				FailureDetailOnionDecode,
+			)
 
-			// As this didn't even clear the link, we don't need to
+			log.Errorf("%v: (hash=%v, pid=%d): %v",
-			// apply an update here since it goes directly to the
+				linkError.FailureDetail, paymentHash, paymentID,
-			// router.
+				err)
-			failureMsg = lnwire.NewTemporaryChannelFailure(nil)
+
+			return linkError
 		}
 
-		return NewForwardingError(failureMsg, 0, userErr)
+		// If we successfully decoded the failure reason, return it.
+		return NewLinkError(failureMsg)
 
 	// A payment had to be timed out on chain before it got past
 	// the first hop. In this case, we'll report a permanent
 	// channel failure as this means us, or the remote party had to
 	// go on chain.
 	case isResolution && htlc.Reason == nil:
-		userErr := fmt.Sprintf("payment was resolved "+
+		linkError := NewDetailedLinkError(
-			"on-chain, then canceled back (hash=%v, pid=%d)",
+			&lnwire.FailPermanentChannelFailure{},
+			FailureDetailOnChainTimeout,
+		)
+
+		log.Info("%v: hash=%v, pid=%d", linkError.FailureDetail,
 			paymentHash, paymentID)
 
-		return NewForwardingError(
+		return linkError
-			&lnwire.FailPermanentChannelFailure{}, 0, userErr,
-		)
 
 	// A regular multi-hop payment error that we'll need to
 	// decrypt.
@@ -1002,18 +1008,21 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error {
 		// selection process. This way we can return the error for
 		// precise link that the sender selected, while optimistically
 		// trying all links to utilize our available bandwidth.
-		linkErrs := make(map[lnwire.ShortChannelID]lnwire.FailureMessage)
+		linkErrs := make(map[lnwire.ShortChannelID]*LinkError)
 
 		// Try to find destination channel link with appropriate
 		// bandwidth.
 		var destination ChannelLink
 		for _, link := range interfaceLinks {
-			var failure lnwire.FailureMessage
+			var failure *LinkError
 
 			// We'll skip any links that aren't yet eligible for
 			// forwarding.
 			if !link.EligibleToForward() {
-				failure = &lnwire.FailUnknownNextPeer{}
+				failure = NewDetailedLinkError(
+					&lnwire.FailUnknownNextPeer{},
+					FailureDetailLinkNotEligible,
+				)
 			} else {
 				// We'll ensure that the HTLC satisfies the
 				// current forwarding conditions of this target
@@ -1048,7 +1057,9 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error {
 				// If we can't find the error of the source,
 				// then we'll return an unknown next peer,
 				// though this should never happen.
-				linkErr = &lnwire.FailUnknownNextPeer{}
+				linkErr = NewLinkError(
+					&lnwire.FailUnknownNextPeer{},
+				)
 				log.Warnf("unable to find err source for "+
 					"outgoing_link=%v, errors=%v",
 					packet.outgoingChanID, newLogClosure(func() string {
@@ -1061,7 +1072,7 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error {
 				htlc.PaymentHash[:], packet.outgoingChanID,
 				linkErr)
 
-			return s.failAddPacket(packet, linkErr, addErr)
+			return s.failAddPacket(packet, linkErr.WireMessage(), addErr)
 		}
 
 		// Send the packet to the destination channel link which

htlcswitch/switch_test.go

@@ -1290,7 +1290,7 @@ func TestSwitchForwardCircuitPersistence(t *testing.T) {
 type multiHopFwdTest struct {
 	name                 string
 	eligible1, eligible2 bool
-	failure1, failure2   lnwire.FailureMessage
+	failure1, failure2   *LinkError
 	expectedReply        lnwire.FailCode
 }
 
@@ -1308,9 +1308,11 @@ func TestSkipIneligibleLinksMultiHopForward(t *testing.T) {
 		// Channel one has a policy failure and the other channel isn't
 		// available.
 		{
-			name:          "policy fail",
+			name:      "policy fail",
-			eligible1:     true,
+			eligible1: true,
-			failure1:      lnwire.NewFinalIncorrectCltvExpiry(0),
+			failure1: NewLinkError(
+				lnwire.NewFinalIncorrectCltvExpiry(0),
+			),
 			expectedReply: lnwire.CodeFinalIncorrectCltvExpiry,
 		},
 
@@ -1325,11 +1327,16 @@ func TestSkipIneligibleLinksMultiHopForward(t *testing.T) {
 		// The requested channel has insufficient bandwidth and the
 		// other channel's policy isn't satisfied.
 		{
-			name:          "non-strict policy fail",
+			name:      "non-strict policy fail",
-			eligible1:     true,
+			eligible1: true,
-			failure1:      lnwire.NewTemporaryChannelFailure(nil),
+			failure1: NewDetailedLinkError(
-			eligible2:     true,
+				lnwire.NewTemporaryChannelFailure(nil),
-			failure2:      lnwire.NewFinalIncorrectCltvExpiry(0),
+				FailureDetailInsufficientBalance,
+			),
+			eligible2: true,
+			failure2: NewLinkError(
+				lnwire.NewFinalIncorrectCltvExpiry(0),
+			),
 			expectedReply: lnwire.CodeTemporaryChannelFailure,
 		},
 	}
@@ -1482,7 +1489,9 @@ func testSkipLinkLocalForward(t *testing.T, eligible bool,
 	aliceChannelLink := newMockChannelLink(
 		s, chanID1, aliceChanID, alicePeer, eligible,
 	)
-	aliceChannelLink.checkHtlcTransitResult = policyResult
+	aliceChannelLink.checkHtlcTransitResult = NewLinkError(
+		policyResult,
+	)
 	if err := s.AddLink(aliceChannelLink); err != nil {
 		t.Fatalf("unable to add alice link: %v", err)
 	}