From 678f416008fefba439a43e007f73f156ccd7e172 Mon Sep 17 00:00:00 2001
From: yyforyongyu <yy2452@columbia.edu>
Date: Tue, 24 Oct 2023 14:53:39 +0800
Subject: [PATCH] routing+docs: make sure non-MPP cannot use skipTempErr

---
 docs/release-notes/release-notes-0.18.0.md |  6 +++
 routing/router.go                          | 11 ++++
 routing/router_test.go                     | 58 +++++++++++++++++++++-
 3 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/docs/release-notes/release-notes-0.18.0.md b/docs/release-notes/release-notes-0.18.0.md
index fa3ac9a3c..b269d8339 100644
--- a/docs/release-notes/release-notes-0.18.0.md
+++ b/docs/release-notes/release-notes-0.18.0.md
@@ -91,6 +91,12 @@
   hash](https://github.com/lightningnetwork/lnd/pull/8106) to the
   signer.SignMessage/signer.VerifyMessage RPCs.
 
+* `sendtoroute` will return an error when it's called using the flag
+  `--skip_temp_err` on a payment that's not a MPP. This is needed as a temp
+  error is defined as a routing error found in one of a MPP's HTLC attempts.
+  If, however, there's only one HTLC attempt, when it's failed, this payment is
+  considered failed, thus there's no such thing as temp error for a non-MPP.
+
 ## lncli Updates
 ## Code Health
 
diff --git a/routing/router.go b/routing/router.go
index f7b20a376..c70a824fa 100644
--- a/routing/router.go
+++ b/routing/router.go
@@ -119,6 +119,10 @@ var (
 	// provided by either a blinded route or a cleartext pubkey.
 	ErrNoTarget = errors.New("destination not set in target or blinded " +
 		"path")
+
+	// ErrSkipTempErr is returned when a non-MPP is made yet the
+	// skipTempErr flag is set.
+	ErrSkipTempErr = errors.New("cannot skip temp error for non-MPP")
 )
 
 // ChannelGraphSource represents the source of information about the topology
@@ -2450,6 +2454,13 @@ func (r *ChannelRouter) sendToRoute(htlcHash lntypes.Hash, rt *route.Route,
 		amt = mpp.TotalMsat()
 	}
 
+	// For non-MPP, there's no such thing as temp error as there's only one
+	// HTLC attempt being made. When this HTLC is failed, the payment is
+	// failed hence cannot be retried.
+	if skipTempErr && mpp == nil {
+		return nil, ErrSkipTempErr
+	}
+
 	// For non-AMP payments the overall payment identifier will be the same
 	// hash as used for this HTLC.
 	paymentIdentifier := htlcHash
diff --git a/routing/router_test.go b/routing/router_test.go
index 67a8964bf..d6abfe7a9 100644
--- a/routing/router_test.go
+++ b/routing/router_test.go
@@ -3384,7 +3384,8 @@ func TestBlockDifferenceFix(t *testing.T) {
 
 	initialBlockHeight := uint32(0)
 
-	// Starting height here is set to 0, which is behind where we want to be.
+	// Starting height here is set to 0, which is behind where we want to
+	// be.
 	ctx := createTestCtxSingleNode(t, initialBlockHeight)
 
 	// Add initial block to our mini blockchain.
@@ -3454,6 +3455,8 @@ func TestBlockDifferenceFix(t *testing.T) {
 
 // TestSendToRouteSkipTempErrSuccess validates a successful payment send.
 func TestSendToRouteSkipTempErrSuccess(t *testing.T) {
+	t.Parallel()
+
 	var (
 		payHash lntypes.Hash
 		payAmt  = lnwire.MilliSatoshi(10000)
@@ -3536,9 +3539,62 @@ func TestSendToRouteSkipTempErrSuccess(t *testing.T) {
 	payment.AssertExpectations(t)
 }
 
+// TestSendToRouteSkipTempErrNonMPP checks that an error is return when
+// skipping temp error for non-MPP.
+func TestSendToRouteSkipTempErrNonMPP(t *testing.T) {
+	t.Parallel()
+
+	var (
+		payHash lntypes.Hash
+		payAmt  = lnwire.MilliSatoshi(10000)
+	)
+
+	node, err := createTestNode()
+	require.NoError(t, err)
+
+	// Create a simple 1-hop route without the MPP field.
+	hops := []*route.Hop{
+		{
+			ChannelID:    1,
+			PubKeyBytes:  node.PubKeyBytes,
+			AmtToForward: payAmt,
+		},
+	}
+	rt, err := route.NewRouteFromHops(payAmt, 100, node.PubKeyBytes, hops)
+	require.NoError(t, err)
+
+	// Create mockers.
+	controlTower := &mockControlTower{}
+	payer := &mockPaymentAttemptDispatcher{}
+	missionControl := &mockMissionControl{}
+
+	// Create the router.
+	router := &ChannelRouter{cfg: &Config{
+		Control:        controlTower,
+		Payer:          payer,
+		MissionControl: missionControl,
+		Clock:          clock.NewTestClock(time.Unix(1, 0)),
+		NextPaymentID: func() (uint64, error) {
+			return 0, nil
+		},
+	}}
+
+	// Expect an error to be returned.
+	attempt, err := router.SendToRouteSkipTempErr(payHash, rt)
+	require.ErrorIs(t, ErrSkipTempErr, err)
+	require.Nil(t, attempt)
+
+	// Assert the above methods are not called.
+	controlTower.AssertExpectations(t)
+	payer.AssertExpectations(t)
+	missionControl.AssertExpectations(t)
+}
+
 // TestSendToRouteSkipTempErrTempFailure validates a temporary failure won't
 // cause the payment to be failed.
 func TestSendToRouteSkipTempErrTempFailure(t *testing.T) {
+	t.Parallel()
+
 	var (
 		payHash lntypes.Hash
 		payAmt  = lnwire.MilliSatoshi(10000)