routing: make sure attempts are always resolved after a timeout

We check the context of the payment lifecycle at the beginning of
the `resumepayment` loop. This will make sure we have always the
latest state of the payment before deciding on the next steps in
the function `decideNextStep`.

routing/payment_lifecycle.go

@@ -226,6 +226,19 @@ func (p *paymentLifecycle) resumePayment(ctx context.Context) ([32]byte,
 	// critical error during path finding.
 lifecycle:
 	for {
+		// Before we attempt any new shard, we'll check to see if we've
+		// gone past the payment attempt timeout or if the context was
+		// canceled. If the context is done, the payment is marked as
+		// failed and we reload the latest payment state to reflect
+		// this.
+		//
+		// NOTE: This can be called several times if there are more
+		// attempts to be resolved after the timeout or context is
+		// cancelled.
+		if err := p.checkContext(ctx); err != nil {
+			return exitWithErr(err)
+		}
+
 		// We update the payment state on every iteration.
 		currentPayment, ps, err := p.reloadPayment()
 		if err != nil {
@@ -241,19 +254,11 @@ lifecycle:
 
 		// We now proceed our lifecycle with the following tasks in
 		// order,
-		//   1. check context.
-		//   2. request route.
-		//   3. create HTLC attempt.
-		//   4. send HTLC attempt.
-		//   5. collect HTLC attempt result.
+		//   1. request route.
+		//   2. create HTLC attempt.
+		//   3. send HTLC attempt.
+		//   4. collect HTLC attempt result.
 		//
-		// Before we attempt any new shard, we'll check to see if we've
-		// gone past the payment attempt timeout, or if the context was
-		// cancelled, or the router is exiting. In any of these cases,
-		// we'll stop this payment attempt short.
-		if err := p.checkContext(ctx); err != nil {
-			return exitWithErr(err)
-		}
 
 		// Now decide the next step of the current lifecycle.
 		step, err := p.decideNextStep(payment)

routing/payment_lifecycle_test.go

@@ -868,25 +868,16 @@ func TestResumePaymentFailOnTimeoutErr(t *testing.T) {
 	// Create a test paymentLifecycle with the initial two calls mocked.
 	p, m := setupTestPaymentLifecycle(t)
 
-	paymentAmt := lnwire.MilliSatoshi(10000)
-
-	// We now enter the payment lifecycle loop.
-	//
-	// 1. calls `FetchPayment` and return the payment.
-	m.control.On("FetchPayment", p.identifier).Return(m.payment, nil).Once()
-
-	// 2. calls `GetState` and return the state.
-	ps := &channeldb.MPPaymentState{
-		RemainingAmt: paymentAmt,
-	}
-	m.payment.On("GetState").Return(ps).Once()
+	// We now enter the payment lifecycle loop, we will check the router
+	// quit channel in the beginning and quit immediately without reloading
+	// the payment.
 
 	// NOTE: GetStatus is only used to populate the logs which is
 	// not critical so we loosen the checks on how many times it's
 	// been called.
 	m.payment.On("GetStatus").Return(channeldb.StatusInFlight)
 
-	// 3. quit the router to return an error.
+	// Quit the router to return an error.
 	close(p.router.quit)
 
 	// Send the payment and assert it failed when router is shutting down.