From 67e40d44335aa19efa17c98bab825f628d7b3361 Mon Sep 17 00:00:00 2001
From: Joost Jager <joost.jager@gmail.com>
Date: Fri, 16 Aug 2019 19:46:51 +0200
Subject: [PATCH] htlcswitch: always assume an onion error for malformed htlc
 failures

Previously a temporary channel failure was returning for unexpected
malformed htlc failures. This is not what we want to communicate to the
sender, because the sender may apply a penalty to us only.

Returning the temporary channel failure is especially problematic if we
ourselves are the sender and the malformed htlc failure comes from our
direct peer. When interpretating the failure, we aren't able to
distinguish anymore between our channel not having enough balance and
our peer sending an unexpected failure back.
---
 htlcswitch/link.go        | 17 +++++++++++++++--
 htlcswitch/switch_test.go |  4 ++--
 lntest/itest/lnd_test.go  |  2 +-
 3 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/htlcswitch/link.go b/htlcswitch/link.go
index 9e9405293..f50cc580d 100644
--- a/htlcswitch/link.go
+++ b/htlcswitch/link.go
@@ -1637,8 +1637,21 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) {
 				OnionSHA256: msg.ShaOnionBlob,
 			}
 		default:
-			log.Errorf("Unknown failure code: %v", msg.FailureCode)
-			failure = &lnwire.FailTemporaryChannelFailure{}
+			log.Warnf("Unexpected failure code received in "+
+				"UpdateFailMailformedHTLC: %v", msg.FailureCode)
+
+			// We don't just pass back the error we received from
+			// our successor. Otherwise we might report a failure
+			// that penalizes us more than needed. If the onion that
+			// we forwarded was correct, the node should have been
+			// able to send back its own failure. The node did not
+			// send back its own failure, so we assume there was a
+			// problem with the onion and report that back. We reuse
+			// the invalid onion key failure because there is no
+			// specific error for this case.
+			failure = &lnwire.FailInvalidOnionKey{
+				OnionSHA256: msg.ShaOnionBlob,
+			}
 		}
 
 		// With the error parsed, we'll convert the into it's opaque
diff --git a/htlcswitch/switch_test.go b/htlcswitch/switch_test.go
index 6e8a1698d..59f6fae9a 100644
--- a/htlcswitch/switch_test.go
+++ b/htlcswitch/switch_test.go
@@ -2097,8 +2097,8 @@ func TestUpdateFailMalformedHTLCErrorConversion(t *testing.T) {
 
 		fwdingErr := err.(*ForwardingError)
 		failureMsg := fwdingErr.FailureMessage
-		if _, ok := failureMsg.(*lnwire.FailTemporaryChannelFailure); !ok {
-			t.Fatalf("expected temp chan failure instead got: %v",
+		if _, ok := failureMsg.(*lnwire.FailInvalidOnionKey); !ok {
+			t.Fatalf("expected onion failure instead got: %v",
 				fwdingErr.FailureMessage)
 		}
 	}
diff --git a/lntest/itest/lnd_test.go b/lntest/itest/lnd_test.go
index ea0996ab2..751edf02c 100644
--- a/lntest/itest/lnd_test.go
+++ b/lntest/itest/lnd_test.go
@@ -3570,7 +3570,7 @@ func testSphinxReplayPersistence(net *lntest.NetworkHarness, t *harnessTest) {
 
 	// Construct the response we expect after sending a duplicate packet
 	// that fails due to sphinx replay detection.
-	replayErr := "TemporaryChannelFailure"
+	replayErr := "InvalidOnionKey"
 	if !strings.Contains(resp.PaymentError, replayErr) {
 		t.Fatalf("received payment error: %v, expected %v",
 			resp.PaymentError, replayErr)