htlcswitch: use distinct error for unreadable failures

htlcswitch/mock.go

@@ -965,3 +965,18 @@ func (m *mockCircuitMap) NumPending() int {
 func (m *mockCircuitMap) NumOpen() int {
 	return 0
 }
+
+type mockOnionErrorDecryptor struct {
+	sourceIdx int
+	message   []byte
+	err       error
+}
+
+func (m *mockOnionErrorDecryptor) DecryptError(encryptedData []byte) (
+	*sphinx.DecryptedError, error) {
+
+	return &sphinx.DecryptedError{
+		SenderIdx: m.sourceIdx,
+		Message:   m.message,
+	}, m.err
+}

htlcswitch/switch.go

@@ -58,6 +58,10 @@ var (
 	// active links in the switch for a specific destination.
 	ErrNoLinksFound = errors.New("no channel links found")
 
+	// ErrUnreadableFailureMessage is returned when the failure message
+	// cannot be decrypted.
+	ErrUnreadableFailureMessage = errors.New("unreadable failure message")
+
 	// zeroPreimage is the empty preimage which is returned when we have
 	// some errors.
 	zeroPreimage [sha256.Size]byte
@@ -936,9 +940,7 @@ func (s *Switch) extractResult(deobfuscator ErrorDecrypter, n *networkResult,
 //    the payment deobfuscator.
 func (s *Switch) parseFailedPayment(deobfuscator ErrorDecrypter,
 	paymentID uint64, paymentHash lntypes.Hash, unencrypted,
-	isResolution bool, htlc *lnwire.UpdateFailHTLC) *ForwardingError {
-
-	var failure *ForwardingError
+	isResolution bool, htlc *lnwire.UpdateFailHTLC) error {
 
 	switch {
 
@@ -960,7 +962,8 @@ func (s *Switch) parseFailedPayment(deobfuscator ErrorDecrypter,
 			// router.
 			failureMsg = lnwire.NewTemporaryChannelFailure(nil)
 		}
-		failure = &ForwardingError{
+
+		return &ForwardingError{
 			FailureSourceIdx: 0,
 			ExtraMsg:         userErr,
 			FailureMessage:   failureMsg,
@@ -974,7 +977,8 @@ func (s *Switch) parseFailedPayment(deobfuscator ErrorDecrypter,
 		userErr := fmt.Sprintf("payment was resolved "+
 			"on-chain, then cancelled back (hash=%v, pid=%d)",
 			paymentHash, paymentID)
-		failure = &ForwardingError{
+
+		return &ForwardingError{
 			FailureSourceIdx: 0,
 			ExtraMsg:         userErr,
 			FailureMessage:   lnwire.FailPermanentChannelFailure{},
@@ -983,24 +987,19 @@ func (s *Switch) parseFailedPayment(deobfuscator ErrorDecrypter,
 	// A regular multi-hop payment error that we'll need to
 	// decrypt.
 	default:
-		var err error
 		// We'll attempt to fully decrypt the onion encrypted
 		// error. If we're unable to then we'll bail early.
-		failure, err = deobfuscator.DecryptError(htlc.Reason)
+		failure, err := deobfuscator.DecryptError(htlc.Reason)
 		if err != nil {
-			userErr := fmt.Sprintf("unable to de-obfuscate "+
-				"onion failure (hash=%v, pid=%d): %v",
+			log.Errorf("unable to de-obfuscate onion failure "+
+				"(hash=%v, pid=%d): %v",
 				paymentHash, paymentID, err)
-			log.Error(userErr)
-			failure = &ForwardingError{
-				FailureSourceIdx: 0,
-				ExtraMsg:         userErr,
-				FailureMessage:   lnwire.NewTemporaryChannelFailure(nil),
-			}
-		}
-	}
 
-	return failure
+			return ErrUnreadableFailureMessage
+		}
+
+		return failure
+	}
 }
 
 // handlePacketForward is used in cases when we need forward the htlc update

htlcswitch/switch_test.go

@@ -2241,3 +2241,107 @@ func TestSwitchGetPaymentResult(t *testing.T) {
 		t.Fatalf("result not received")
 	}
 }
+
+// TestInvalidFailure tests that the switch returns an unreadable failure error
+// if the failure cannot be decrypted.
+func TestInvalidFailure(t *testing.T) {
+	t.Parallel()
+
+	alicePeer, err := newMockServer(t, "alice", testStartingHeight, nil, 6)
+	if err != nil {
+		t.Fatalf("unable to create alice server: %v", err)
+	}
+
+	s, err := initSwitchWithDB(testStartingHeight, nil)
+	if err != nil {
+		t.Fatalf("unable to init switch: %v", err)
+	}
+	if err := s.Start(); err != nil {
+		t.Fatalf("unable to start switch: %v", err)
+	}
+	defer s.Stop()
+
+	chanID1, _, aliceChanID, _ := genIDs()
+
+	// Set up a mock channel link.
+	aliceChannelLink := newMockChannelLink(
+		s, chanID1, aliceChanID, alicePeer, true,
+	)
+	if err := s.AddLink(aliceChannelLink); err != nil {
+		t.Fatalf("unable to add link: %v", err)
+	}
+
+	// Create a request which should be forwarded to the mock channel link.
+	preimage, err := genPreimage()
+	if err != nil {
+		t.Fatalf("unable to generate preimage: %v", err)
+	}
+	rhash := fastsha256.Sum256(preimage[:])
+	update := &lnwire.UpdateAddHTLC{
+		PaymentHash: rhash,
+		Amount:      1,
+	}
+
+	paymentID := uint64(123)
+
+	// Send the request.
+	err = s.SendHTLC(
+		aliceChannelLink.ShortChanID(), paymentID, update,
+	)
+	if err != nil {
+		t.Fatalf("unable to send payment: %v", err)
+	}
+
+	// Catch the packet and complete the circuit so that the switch is ready
+	// for a response.
+	select {
+	case packet := <-aliceChannelLink.packets:
+		if err := aliceChannelLink.completeCircuit(packet); err != nil {
+			t.Fatalf("unable to complete payment circuit: %v", err)
+		}
+
+	case <-time.After(time.Second):
+		t.Fatal("request was not propagated to destination")
+	}
+
+	// Send response packet with an unreadable failure message to the
+	// switch. The reason failed is not relevant, because we mock the
+	// decryption.
+	packet := &htlcPacket{
+		outgoingChanID: aliceChannelLink.ShortChanID(),
+		outgoingHTLCID: 0,
+		amount:         1,
+		htlc: &lnwire.UpdateFailHTLC{
+			Reason: []byte{1, 2, 3},
+		},
+	}
+
+	if err := s.forward(packet); err != nil {
+		t.Fatalf("can't forward htlc packet: %v", err)
+	}
+
+	// Get payment result from switch. We expect an unreadable failure
+	// message error.
+	deobfuscator := SphinxErrorDecrypter{
+		OnionErrorDecrypter: &mockOnionErrorDecryptor{
+			err: ErrUnreadableFailureMessage,
+		},
+	}
+
+	resultChan, err := s.GetPaymentResult(
+		paymentID, rhash, &deobfuscator,
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	select {
+	case result := <-resultChan:
+		if result.Error != ErrUnreadableFailureMessage {
+			t.Fatal("expected unreadable failure message")
+		}
+
+	case <-time.After(time.Second):
+		t.Fatal("err wasn't received")
+	}
+}