From a979b91b27d6533db8c02862c73b79aec3f065df Mon Sep 17 00:00:00 2001
From: "Johan T. Halseth" <johanth@gmail.com>
Date: Wed, 1 Apr 2020 00:13:24 +0200
Subject: [PATCH] routing: remove errNoRoute and lastError

Now that SendToRoute is no longer using the payment lifecycle, we
remove the error structs and vars used to cache the last encountered
error. For SendToRoute this will now be returned directly after a shard
has failed.

For SendPayment this means that the last error encountered durinng
pathfinding no longer will be returned. All errors encounterd can
instead be inspected from the HTLC list.
---
 routing/payment_lifecycle.go | 27 ---------------------------
 routing/router_test.go       | 25 ++++++++++++++++++++++++-
 2 files changed, 24 insertions(+), 28 deletions(-)

diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go
index c386c49f0..40089479f 100644
--- a/routing/payment_lifecycle.go
+++ b/routing/payment_lifecycle.go
@@ -13,20 +13,6 @@ import (
 	"github.com/lightningnetwork/lnd/routing/route"
 )
 
-// errNoRoute is returned when all routes from the payment session have been
-// attempted.
-type errNoRoute struct {
-	// lastError is the error encountered during the last payment attempt,
-	// if at least one attempt has been made.
-	lastError error
-}
-
-// Error returns a string representation of the error.
-func (e errNoRoute) Error() string {
-	return fmt.Sprintf("unable to route payment to destination: %v",
-		e.lastError)
-}
-
 // paymentLifecycle holds all information about the current state of a payment
 // needed to resume if from any point.
 type paymentLifecycle struct {
@@ -146,14 +132,6 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) {
 					return [32]byte{}, nil, saveErr
 				}
 
-				// If there was an error already recorded for this
-				// payment, we'll return that.
-				if shardHandler.lastError != nil {
-					return [32]byte{}, nil, errNoRoute{
-						lastError: shardHandler.lastError,
-					}
-				}
-
 				// Terminal state, return.
 				return [32]byte{}, nil, err
 			}
@@ -232,8 +210,6 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) {
 type shardHandler struct {
 	paymentHash lntypes.Hash
 	router      *ChannelRouter
-
-	lastError error
 }
 
 // launchOutcome is a type returned from launchShard that indicates whether the
@@ -542,9 +518,6 @@ func (p *shardHandler) handleSendError(attempt *channeldb.HTLCAttemptInfo,
 		attempt.AttemptID, &attempt.Route, sendErr,
 	)
 	if reason == nil {
-		// Save the forwarding error so it can be returned if
-		// this turns out to be the last attempt.
-		p.lastError = sendErr
 		return nil
 	}
 
diff --git a/routing/router_test.go b/routing/router_test.go
index d8550ac73..d3c866ae9 100644
--- a/routing/router_test.go
+++ b/routing/router_test.go
@@ -792,10 +792,33 @@ func TestSendPaymentErrorPathPruning(t *testing.T) {
 
 	// The final error returned should also indicate that the peer wasn't
 	// online (the last error we returned).
-	if !strings.Contains(err.Error(), "UnknownNextPeer") {
+	// TODO: proper err code
+	if !strings.Contains(err.Error(), "unable to find") {
 		t.Fatalf("expected UnknownNextPeer instead got: %v", err)
 	}
 
+	// Inspect the two attempts that were made before the payment failed.
+	p, err := ctx.router.cfg.Control.FetchPayment(payHash)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(p.HTLCs) != 2 {
+		t.Fatalf("expected two attempts got %v", len(p.HTLCs))
+	}
+
+	// We expect the first attempt to have failed with a
+	// TemporaryChannelFailure, the second with UnknownNextPeer.
+	msg := p.HTLCs[0].Failure.Message
+	if _, ok := msg.(*lnwire.FailTemporaryChannelFailure); !ok {
+		t.Fatalf("unexpected fail message: %T", msg)
+	}
+
+	msg = p.HTLCs[1].Failure.Message
+	if _, ok := msg.(*lnwire.FailUnknownNextPeer); !ok {
+		t.Fatalf("unexpected fail message: %T", msg)
+	}
+
 	ctx.router.cfg.MissionControl.(*MissionControl).ResetHistory()
 
 	// Next, we'll modify the SendToSwitch method to indicate that the