From d08e8ddd614a30fec002b927131eed0b8de233db Mon Sep 17 00:00:00 2001
From: Conner Fromknecht <conner@lightning.engineering>
Date: Wed, 30 Oct 2019 21:20:08 -0700
Subject: [PATCH] routing/result_interpretation: process InvalidOnionPayload

An InvalidOnionPayload implies that the onion was successfully received
by the reporting node, but that they were unable to extract the
contents. Since we assume our own behavior is correct, this mostly
likely poins to an error in the reporter's implementation or that we
sent an unknown required type. Therefore we only penalize that single
hop, and consider the failure terminal if the receiver reported it.
---
 routing/result_interpretation.go      | 18 +++++++
 routing/result_interpretation_test.go | 69 +++++++++++++++++++++++++++
 2 files changed, 87 insertions(+)

diff --git a/routing/result_interpretation.go b/routing/result_interpretation.go
index 643d4df0..8f193048 100644
--- a/routing/result_interpretation.go
+++ b/routing/result_interpretation.go
@@ -270,6 +270,16 @@ func (i *interpretedResult) processPaymentOutcomeIntermediate(
 		}
 	}
 
+	reportNode := func() {
+		// Fail only the node that reported the failure.
+		i.failNode(route, errorSourceIdx)
+
+		// Other preceding channels in the route forwarded correctly.
+		if errorSourceIdx > 1 {
+			i.successPairRange(route, 0, errorSourceIdx-2)
+		}
+	}
+
 	reportAll := func() {
 		// We trust ourselves. If the error comes from the first hop, we
 		// can penalize the whole node. In that case there is no
@@ -302,6 +312,14 @@ func (i *interpretedResult) processPaymentOutcomeIntermediate(
 
 		reportOutgoing()
 
+	// If InvalidOnionPayload is received, we penalize only the reporting
+	// node. We know the preceding hop didn't corrupt the onion, since the
+	// reporting node is able to send the failure. We assume that we
+	// constructed a valid onion payload and that the failure is most likely
+	// an unknown required type or a bug in their implementation.
+	case *lnwire.InvalidOnionPayload:
+		reportNode()
+
 	// If the next hop in the route wasn't known or offline, we'll only
 	// penalize the channel set which we attempted to route over. This is
 	// conservative, and it can handle faulty channels between nodes
diff --git a/routing/result_interpretation_test.go b/routing/result_interpretation_test.go
index 1bf0fa90..4f3fd250 100644
--- a/routing/result_interpretation_test.go
+++ b/routing/result_interpretation_test.go
@@ -197,6 +197,75 @@ var resultTestCases = []resultTestCase{
 			policyFailure: getPolicyFailure(2, 3),
 		},
 	},
+
+	// Tests an invalid onion payload from a final hop. The final hop should
+	// be failed while the proceeding hops are reproed as successes. The
+	// failure is terminal since the receiver can't process our onion.
+	{
+		name:          "fail invalid onion payload final hop",
+		route:         &routeFourHop,
+		failureSrcIdx: 4,
+		failure:       lnwire.NewInvalidOnionPayload(0, 0),
+
+		expectedResult: &interpretedResult{
+			pairResults: map[DirectedNodePair]pairResult{
+				getTestPair(0, 1): {
+					success: true,
+				},
+				getTestPair(1, 2): {
+					success: true,
+				},
+				getTestPair(2, 3): {
+					success: true,
+				},
+				getTestPair(4, 3): {},
+			},
+			finalFailureReason: &reasonError,
+			nodeFailure:        &hops[4],
+		},
+	},
+
+	// Tests an invalid onion payload from an intermediate hop. Only the
+	// reporting node should be failed. The failure is non-terminal since we
+	// can still try other paths.
+	{
+		name:          "fail invalid onion payload intermediate",
+		route:         &routeFourHop,
+		failureSrcIdx: 3,
+		failure:       lnwire.NewInvalidOnionPayload(0, 0),
+
+		expectedResult: &interpretedResult{
+			pairResults: map[DirectedNodePair]pairResult{
+				getTestPair(0, 1): {
+					success: true,
+				},
+				getTestPair(1, 2): {
+					success: true,
+				},
+				getTestPair(3, 2): {},
+				getTestPair(3, 4): {},
+			},
+			nodeFailure: &hops[3],
+		},
+	},
+
+	// Tests an invalid onion payload in a direct peer that is also the
+	// final hop. The final node should be failed and the error is terminal
+	// since the remote node can't process our onion.
+	{
+		name:          "fail invalid onion payload direct",
+		route:         &routeOneHop,
+		failureSrcIdx: 1,
+		failure:       lnwire.NewInvalidOnionPayload(0, 0),
+
+		expectedResult: &interpretedResult{
+			pairResults: map[DirectedNodePair]pairResult{
+				getTestPair(1, 0): {},
+			},
+			finalFailureReason: &reasonError,
+			nodeFailure:        &hops[1],
+		},
+	},
 }
 
 // TestResultInterpretation executes a list of test cases that test the result