From 173900c8dcb7829348cdbb7f638ff6ba7d37c20b Mon Sep 17 00:00:00 2001
From: yyforyongyu <yy2452@columbia.edu>
Date: Mon, 13 Feb 2023 18:32:02 +0800
Subject: [PATCH] routing: only fail attempt inside `handleSwitchErr`

This commit makes sure we only fail attempt inside `handleSwitchErr` to
ensure the orders in failing payment and attempts. It refactors
`collectResult` to return `attemptResult`, and expands `handleSwitchErr`
to also handle the case where the attemptID is not found.
---
 routing/payment_lifecycle.go | 56 ++++++++++++------------------------
 1 file changed, 19 insertions(+), 37 deletions(-)

diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go
index 6cbe5d69c..20497c62a 100644
--- a/routing/payment_lifecycle.go
+++ b/routing/payment_lifecycle.go
@@ -413,32 +413,15 @@ func (p *paymentLifecycle) collectResultAsync(attempt *channeldb.HTLCAttempt) {
 		defer handleResultErr()
 
 		// Block until the result is available.
-		result, err := p.collectResult(attempt)
+		_, err := p.collectResult(attempt)
 		if err != nil {
-			if err != ErrRouterShuttingDown &&
-				err != htlcswitch.ErrSwitchExiting &&
-				err != errShardHandlerExiting {
+			log.Errorf("Error collecting result for attempt %v "+
+				"in payment %v: %v", attempt.AttemptID,
+				p.identifier, err)
 
-				log.Errorf("Error collecting result for "+
-					"shard %v for payment %v: %v",
-					attempt.AttemptID, p.identifier, err)
-			}
-
-			// Overwrite the param errToSend and return so that the
-			// defer function will use the param to proceed.
 			errToSend = err
 			return
 		}
-
-		// If a non-critical error was encountered handle it and mark
-		// the payment failed if the failure was terminal.
-		if result.err != nil {
-			// Overwrite the param errToSend and return so that the
-			// defer function will use the param to proceed. Notice
-			// that the errToSend could be nil here.
-			_, errToSend = p.handleSwitchErr(attempt, result.err)
-			return
-		}
 	}()
 }
 
@@ -477,24 +460,12 @@ func (p *paymentLifecycle) collectResult(attempt *channeldb.HTLCAttempt) (
 	resultChan, err := p.router.cfg.Payer.GetAttemptResult(
 		attempt.AttemptID, p.identifier, errorDecryptor,
 	)
-	switch {
-	// If this attempt ID is unknown to the Switch, it means it was never
-	// checkpointed and forwarded by the switch before a restart. In this
-	// case we can safely send a new payment attempt, and wait for its
-	// result to be available.
-	case err == htlcswitch.ErrPaymentIDNotFound:
-		log.Debugf("Attempt ID %v for payment %v not found in "+
-			"the Switch, retrying.", attempt.AttemptID,
-			p.identifier)
-
-		return p.failAttempt(attempt.AttemptID, err)
-
-	// A critical, unexpected error was encountered.
-	case err != nil:
+	// Handle the switch error.
+	if err != nil {
 		log.Errorf("Failed getting result for attemptID %d "+
 			"from switch: %v", attempt.AttemptID, err)
 
-		return nil, err
+		return p.handleSwitchErr(attempt, err)
 	}
 
 	// The switch knows about this payment, we'll wait for a result to be
@@ -517,7 +488,7 @@ func (p *paymentLifecycle) collectResult(attempt *channeldb.HTLCAttempt) (
 	// In case of a payment failure, fail the attempt with the control
 	// tower and return.
 	if result.Error != nil {
-		return p.failAttempt(attempt.AttemptID, result.Error)
+		return p.handleSwitchErr(attempt, result.Error)
 	}
 
 	// We successfully got a payment result back from the switch.
@@ -754,6 +725,17 @@ func (p *paymentLifecycle) handleSwitchErr(attempt *channeldb.HTLCAttempt,
 		return p.failPaymentAndAttempt(attemptID, reason, sendErr)
 	}
 
+	// If this attempt ID is unknown to the Switch, it means it was never
+	// checkpointed and forwarded by the switch before a restart. In this
+	// case we can safely send a new payment attempt, and wait for its
+	// result to be available.
+	if errors.Is(sendErr, htlcswitch.ErrPaymentIDNotFound) {
+		log.Debugf("Attempt ID %v for payment %v not found in the "+
+			"Switch, retrying.", attempt.AttemptID, p.identifier)
+
+		return p.failAttempt(attemptID, sendErr)
+	}
+
 	if sendErr == htlcswitch.ErrUnreadableFailureMessage {
 		log.Warn("Unreadable failure when sending htlc: id=%v, hash=%v",
 			attempt.AttemptID, attempt.Hash)