Merge pull request #6408 from Roasbeef/v0.14.3-branch-rc2

release: create v0.14.3-rc2 branch

build/version.go

@@ -48,7 +48,7 @@ const (
 
 	// AppPreRelease MUST only contain characters from semanticAlphabet
 	// per the semantic versioning spec.
-	AppPreRelease = "beta.rc1"
+	AppPreRelease = "beta.rc2"
 )
 
 func init() {

docs/release-notes/release-notes-0.14.3.md

@@ -1,5 +1,9 @@
 # Release Notes
 
+## RPC Server
+
+* [Support for making routes with the legacy onion payload format via `SendToRoute` has been removed.](https://github.com/lightningnetwork/lnd/pull/6385)
+
 ## Bug fixes
 
 * The REST proxy (`grpc-gateway` library) had a fallback that redirected `POST`
@@ -26,6 +30,9 @@
   party sweeps our anchor
   output](https://github.com/lightningnetwork/lnd/pull/6274).
 
+* [Fixed race condition resulting in MPP payments sometimes getting stuck
+  in-flight](https://github.com/lightningnetwork/lnd/pull/6352).
+
 ## Code Health
 
 ### Code cleanup, refactor, typo fixes
@@ -35,7 +42,7 @@
   code external to lnd to call the function, where previously it would require
   access to lnd's internals.
 
-## Misc
+## Clustering
 
 * [Make etcd leader election session
   TTL](https://github.com/lightningnetwork/lnd/pull/6342) configurable.

lnrpc/lightning.proto

@@ -2718,7 +2718,7 @@ message Hop {
     TLV format. Note that if any custom tlv_records below are specified, then
     this field MUST be set to true for them to be encoded properly.
     */
-    bool tlv_payload = 9;
+    bool tlv_payload = 9 [deprecated = true];
 
     /*
     An optional TLV record that signals the use of an MPP payment. If present,

lnrpc/routerrpc/router_backend.go

@@ -476,7 +476,7 @@ func UnmarshallHopWithPubkey(rpcHop *lnrpc.Hop, pubkey route.Vertex) (*route.Hop
 		PubKeyBytes:      pubkey,
 		ChannelID:        rpcHop.ChanId,
 		CustomRecords:    customRecords,
-		LegacyPayload:    !rpcHop.TlvPayload,
+		LegacyPayload:    false,
 		MPP:              mpp,
 		AMP:              amp,
 	}, nil

routing/payment_lifecycle.go

@@ -600,9 +600,6 @@ func (p *shardHandler) collectResult(attempt *channeldb.HTLCAttemptInfo) (
 
 	case <-p.router.quit:
 		return nil, ErrRouterShuttingDown
-
-	case <-p.quit:
-		return nil, errShardHandlerExiting
 	}
 
 	// In case of a payment failure, fail the attempt with the control

routing/router_test.go

@@ -4187,37 +4187,46 @@ func TestSendMPPaymentFailedWithShardsInFlight(t *testing.T) {
 	// Create a buffered chan and it will be returned by GetPaymentResult.
 	payer.resultChan = make(chan *htlcswitch.PaymentResult, 10)
 
-	// We use the failAttemptCount to track how many attempts we want to
-	// fail. Each time the following mock method is called, the count gets
-	// updated.
-	failAttemptCount := 0
+	// We use the getPaymentResultCnt to track how many times we called
+	// GetPaymentResult. As shard launch is sequential, and we fail the
+	// first shard that calls GetPaymentResult, we may end up with different
+	// counts since the lifecycle itself is asynchronous. To avoid flakes
+	// due to this undeterminsitic behavior, we'll compare the final
+	// getPaymentResultCnt with other counters to create a final test
+	// expectation.
+	getPaymentResultCnt := 0
 	payer.On("GetPaymentResult",
 		mock.Anything, identifier, mock.Anything,
 	).Run(func(args mock.Arguments) {
 		// Before the mock method is returned, we send the result to
 		// the read-only chan.
 
 		// Update the counter.
-		failAttemptCount++
+		getPaymentResultCnt++
 
 		// We fail the first attempt with terminal error.
-		if failAttemptCount == 1 {
+		if getPaymentResultCnt == 1 {
 			payer.resultChan <- &htlcswitch.PaymentResult{
 				Error: htlcswitch.NewForwardingError(
 					&lnwire.FailIncorrectDetails{},
 					1,
 				),
 			}
 			return
-
 		}
 
-		// For the rest attempts we will NOT send anything to the
-		// resultChan, thus making all the shards in active state,
-		// neither settled or failed.
+		// For the rest of the attempts we'll simulate that a network
+		// result update_fail_htlc has been received. This way the
+		// payment will fail cleanly.
+		payer.resultChan <- &htlcswitch.PaymentResult{
+			Error: htlcswitch.NewForwardingError(
+				&lnwire.FailTemporaryChannelFailure{},
+				1,
+			),
+		}
 	})
 
-	// Mock the FailAttempt method to fail EXACTLY once.
+	// Mock the FailAttempt method to fail (at least once).
 	var failedAttempt channeldb.HTLCAttempt
 	controlTower.On("FailAttempt",
 		identifier, mock.Anything, mock.Anything,
@@ -4227,22 +4236,27 @@ func TestSendMPPaymentFailedWithShardsInFlight(t *testing.T) {
 		failedAttempt = payment.HTLCs[0]
 		failedAttempt.Failure = &channeldb.HTLCFailInfo{}
 		payment.HTLCs[0] = failedAttempt
-	}).Once()
+	})
 
 	// Setup ReportPaymentFail to return nil reason and error so the
 	// payment won't fail.
 	failureReason := channeldb.FailureReasonPaymentDetails
+	cntReportPaymentFail := 0
 	missionControl.On("ReportPaymentFail",
 		mock.Anything, mock.Anything, mock.Anything, mock.Anything,
 	).Return(&failureReason, nil).Run(func(args mock.Arguments) {
 		payment.FailureReason = &failureReason
-	}).Once()
+		cntReportPaymentFail++
+	})
 
 	// Simple mocking the rest.
-	controlTower.On("Fail", identifier, failureReason).Return(nil).Once()
+	cntFail := 0
+	controlTower.On("Fail", identifier, failureReason).Return(nil)
 	payer.On("SendHTLC",
 		mock.Anything, mock.Anything, mock.Anything,
-	).Return(nil)
+	).Return(nil).Run(func(args mock.Arguments) {
+		cntFail++
+	})
 
 	// Call the actual method SendPayment on router. This is place inside a
 	// goroutine so we can set a timeout for the whole test, in case
@@ -4264,6 +4278,9 @@ func TestSendMPPaymentFailedWithShardsInFlight(t *testing.T) {
 	// methods are called as expected.
 	require.Error(t, err, "expected send payment error")
 	require.EqualValues(t, [32]byte{}, p, "preimage not match")
+	require.GreaterOrEqual(t, getPaymentResultCnt, 1)
+	require.Equal(t, getPaymentResultCnt, cntReportPaymentFail)
+	require.Equal(t, getPaymentResultCnt, cntFail)
 
 	controlTower.AssertExpectations(t)
 	payer.AssertExpectations(t)