Merge pull request #8170 from yyforyongyu/0-18-staging-missed

Add missing commits from `0-18-staging`

channeldb/mp_payment.go

@@ -219,10 +219,10 @@ func (m *MPPayment) Terminated() bool {
 // TerminalInfo returns any HTLC settle info recorded. If no settle info is
 // recorded, any payment level failure will be returned. If neither a settle
 // nor a failure is recorded, both return values will be nil.
-func (m *MPPayment) TerminalInfo() (*HTLCSettleInfo, *FailureReason) {
+func (m *MPPayment) TerminalInfo() (*HTLCAttempt, *FailureReason) {
 	for _, h := range m.HTLCs {
 		if h.Settle != nil {
-			return h.Settle, nil
+			return &h, nil
 		}
 	}
 
@@ -264,6 +264,7 @@ func (m *MPPayment) InFlightHTLCs() []HTLCAttempt {
 
 // GetAttempt returns the specified htlc attempt on the payment.
 func (m *MPPayment) GetAttempt(id uint64) (*HTLCAttempt, error) {
+	// TODO(yy): iteration can be slow, make it into a tree or use BS.
 	for _, htlc := range m.HTLCs {
 		htlc := htlc
 		if htlc.AttemptID == id {
@@ -463,9 +464,49 @@ func (m *MPPayment) GetHTLCs() []HTLCAttempt {
 	return m.HTLCs
 }
 
-// GetFailureReason returns the failure reason.
-func (m *MPPayment) GetFailureReason() *FailureReason {
-	return m.FailureReason
+// AllowMoreAttempts is used to decide whether we can safely attempt more HTLCs
+// for a given payment state. Return an error if the payment is in an
+// unexpected state.
+func (m *MPPayment) AllowMoreAttempts() (bool, error) {
+	// Now check whether the remainingAmt is zero or not. If we don't have
+	// any remainingAmt, no more HTLCs should be made.
+	if m.State.RemainingAmt == 0 {
+		// If the payment is newly created, yet we don't have any
+		// remainingAmt, return an error.
+		if m.Status == StatusInitiated {
+			return false, fmt.Errorf("%w: initiated payment has "+
+				"zero remainingAmt", ErrPaymentInternal)
+		}
+
+		// Otherwise, exit early since all other statuses with zero
+		// remainingAmt indicate no more HTLCs can be made.
+		return false, nil
+	}
+
+	// Otherwise, the remaining amount is not zero, we now decide whether
+	// to make more attempts based on the payment's current status.
+	//
+	// If at least one of the payment's attempts is settled, yet we haven't
+	// sent all the amount, it indicates something is wrong with the peer
+	// as the preimage is received. In this case, return an error state.
+	if m.Status == StatusSucceeded {
+		return false, fmt.Errorf("%w: payment already succeeded but "+
+			"still have remaining amount %v", ErrPaymentInternal,
+			m.State.RemainingAmt)
+	}
+
+	// Now check if we can register a new HTLC.
+	err := m.Registrable()
+	if err != nil {
+		log.Warnf("Payment(%v): cannot register HTLC attempt: %v, "+
+			"current status: %s", m.Info.PaymentIdentifier,
+			err, m.Status)
+
+		return false, nil
+	}
+
+	// Now we know we can register new HTLCs.
+	return true, nil
 }
 
 // serializeHTLCSettleInfo serializes the details of a settled htlc.

channeldb/mp_payment_test.go

@@ -368,6 +368,183 @@ func TestNeedWaitAttempts(t *testing.T) {
 	}
 }
 
+// TestAllowMoreAttempts checks whether more attempts can be created against
+// ALL possible payment statuses.
+func TestAllowMoreAttempts(t *testing.T) {
+	t.Parallel()
+
+	testCases := []struct {
+		status         PaymentStatus
+		remainingAmt   lnwire.MilliSatoshi
+		hasSettledHTLC bool
+		paymentFailed  bool
+		allowMore      bool
+		expectedErr    error
+	}{
+		{
+			// A newly created payment with zero remainingAmt
+			// indicates an error.
+			status:       StatusInitiated,
+			remainingAmt: 0,
+			allowMore:    false,
+			expectedErr:  ErrPaymentInternal,
+		},
+		{
+			// With zero remainingAmt we don't allow more HTLC
+			// attempts.
+			status:       StatusInFlight,
+			remainingAmt: 0,
+			allowMore:    false,
+			expectedErr:  nil,
+		},
+		{
+			// With zero remainingAmt we don't allow more HTLC
+			// attempts.
+			status:       StatusSucceeded,
+			remainingAmt: 0,
+			allowMore:    false,
+			expectedErr:  nil,
+		},
+		{
+			// With zero remainingAmt we don't allow more HTLC
+			// attempts.
+			status:       StatusFailed,
+			remainingAmt: 0,
+			allowMore:    false,
+			expectedErr:  nil,
+		},
+		{
+			// With zero remainingAmt and settled HTLCs we don't
+			// allow more HTLC attempts.
+			status:         StatusInFlight,
+			remainingAmt:   0,
+			hasSettledHTLC: true,
+			allowMore:      false,
+			expectedErr:    nil,
+		},
+		{
+			// With zero remainingAmt and failed payment we don't
+			// allow more HTLC attempts.
+			status:        StatusInFlight,
+			remainingAmt:  0,
+			paymentFailed: true,
+			allowMore:     false,
+			expectedErr:   nil,
+		},
+		{
+			// With zero remainingAmt and both settled HTLCs and
+			// failed payment, we don't allow more HTLC attempts.
+			status:         StatusInFlight,
+			remainingAmt:   0,
+			hasSettledHTLC: true,
+			paymentFailed:  true,
+			allowMore:      false,
+			expectedErr:    nil,
+		},
+		{
+			// A newly created payment can have more attempts.
+			status:       StatusInitiated,
+			remainingAmt: 1000,
+			allowMore:    true,
+			expectedErr:  nil,
+		},
+		{
+			// With HTLCs inflight we can have more attempts when
+			// the remainingAmt is not zero and we have neither
+			// failed payment or settled HTLCs.
+			status:       StatusInFlight,
+			remainingAmt: 1000,
+			allowMore:    true,
+			expectedErr:  nil,
+		},
+		{
+			// With HTLCs inflight we cannot have more attempts
+			// though the remainingAmt is not zero but we have
+			// settled HTLCs.
+			status:         StatusInFlight,
+			remainingAmt:   1000,
+			hasSettledHTLC: true,
+			allowMore:      false,
+			expectedErr:    nil,
+		},
+		{
+			// With HTLCs inflight we cannot have more attempts
+			// though the remainingAmt is not zero but we have
+			// failed payment.
+			status:        StatusInFlight,
+			remainingAmt:  1000,
+			paymentFailed: true,
+			allowMore:     false,
+			expectedErr:   nil,
+		},
+		{
+			// With HTLCs inflight we cannot have more attempts
+			// though the remainingAmt is not zero but we have
+			// settled HTLCs and failed payment.
+			status:         StatusInFlight,
+			remainingAmt:   1000,
+			hasSettledHTLC: true,
+			paymentFailed:  true,
+			allowMore:      false,
+			expectedErr:    nil,
+		},
+		{
+			// With the payment settled, but the remainingAmt is
+			// not zero, we have an error state.
+			status:         StatusSucceeded,
+			remainingAmt:   1000,
+			hasSettledHTLC: true,
+			allowMore:      false,
+			expectedErr:    ErrPaymentInternal,
+		},
+		{
+			// With the payment failed with no inflight HTLCs, we
+			// don't allow more attempts to be made.
+			status:        StatusFailed,
+			remainingAmt:  1000,
+			paymentFailed: true,
+			allowMore:     false,
+			expectedErr:   nil,
+		},
+		{
+			// With the payment in an unknown state, we don't allow
+			// more attempts to be made.
+			status:       0,
+			remainingAmt: 1000,
+			allowMore:    false,
+			expectedErr:  nil,
+		},
+	}
+
+	for i, tc := range testCases {
+		tc := tc
+
+		p := &MPPayment{
+			Info: &PaymentCreationInfo{
+				PaymentIdentifier: [32]byte{1, 2, 3},
+			},
+			Status: tc.status,
+			State: &MPPaymentState{
+				RemainingAmt:   tc.remainingAmt,
+				HasSettledHTLC: tc.hasSettledHTLC,
+				PaymentFailed:  tc.paymentFailed,
+			},
+		}
+
+		name := fmt.Sprintf("test_%d|status=%s|remainingAmt=%v", i,
+			tc.status, tc.remainingAmt)
+
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			result, err := p.AllowMoreAttempts()
+			require.ErrorIs(t, err, tc.expectedErr)
+			require.Equalf(t, tc.allowMore, result, "status=%v, "+
+				"remainingAmt=%v", tc.status, tc.remainingAmt)
+		})
+	}
+}
+
 func makeActiveAttempt(total, fee int) HTLCAttempt {
 	return HTLCAttempt{
 		HTLCAttemptInfo: makeAttemptInfo(total, total-fee),

docs/release-notes/release-notes-0.18.0.md

@@ -37,6 +37,9 @@
   might panic due to empty witness data found in a transaction. More details
   can be found [here](https://github.com/bitcoin/bitcoin/issues/28730).
 
+* [Fixed a case](https://github.com/lightningnetwork/lnd/pull/7503) where it's
+  possible a failed payment might be stuck in pending.
+
 # New Features
 ## Functional Enhancements
 
@@ -58,6 +61,13 @@
   [http-header-timeout](https://github.com/lightningnetwork/lnd/pull/7715), is added so users can specify the amount of time the http server will wait for a request to complete before closing the connection. The default value is 5 seconds.
 
 ## RPC Additions
+
+* [Deprecated](https://github.com/lightningnetwork/lnd/pull/7175)
+  `StatusUnknown` from the payment's rpc response in its status and added a new
+  status, `StatusInitiated`, to explicitly report its current state. Before
+  running this new version, please make sure to upgrade your client application
+  to include this new status so it can understand the RPC response properly.
+
 ## lncli Additions
 
 # Improvements
@@ -81,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
 
@@ -90,6 +106,11 @@
   `lnrpc.GetInfoResponse` message along with the `chain` field in the
   `lnrpc.Chain` message have also been deprecated for the same reason.
 
+* The payment lifecycle code has been refactored to improve its maintainablity.
+  In particular, the complexity involved in the lifecycle loop has been
+  decoupled into logical steps, with each step having its own responsibility,
+  making it easier to reason about the payment flow.
+
 ## Breaking Changes
 ## Performance Improvements
 

htlcswitch/switch.go

@@ -1000,6 +1000,8 @@ func (s *Switch) extractResult(deobfuscator ErrorDecrypter, n *networkResult,
 	// We've received a fail update which means we can finalize the
 	// user payment and return fail response.
 	case *lnwire.UpdateFailHTLC:
+		// TODO(yy): construct deobfuscator here to avoid creating it
+		// in paymentLifecycle even for settled HTLCs.
 		paymentErr := s.parseFailedPayment(
 			deobfuscator, attemptID, paymentHash, n.unencrypted,
 			n.isResolution, htlc,

itest/lnd_max_htlcs_test.go

@@ -122,8 +122,7 @@ func acceptHoldInvoice(ht *lntest.HarnessTest, idx int, sender,
 	invoice := receiver.RPC.AddHoldInvoice(req)
 
 	invStream := receiver.RPC.SubscribeSingleInvoice(hash[:])
-	inv := ht.ReceiveSingleInvoice(invStream)
-	require.Equal(ht, lnrpc.Invoice_OPEN, inv.State, "expect open")
+	ht.AssertInvoiceState(invStream, lnrpc.Invoice_OPEN)
 
 	sendReq := &routerrpc.SendPaymentRequest{
 		PaymentRequest: invoice.PaymentRequest,
@@ -145,9 +144,7 @@ func acceptHoldInvoice(ht *lntest.HarnessTest, idx int, sender,
 	)
 	require.Len(ht, payment.Htlcs, 1)
 
-	inv = ht.ReceiveSingleInvoice(invStream)
-	require.Equal(ht, lnrpc.Invoice_ACCEPTED, inv.State,
-		"expected accepted")
+	ht.AssertInvoiceState(invStream, lnrpc.Invoice_ACCEPTED)
 
 	return &holdSubscription{
 		recipient:           receiver,

routing/control_tower.go

@@ -31,8 +31,14 @@ type dbMPPayment interface {
 	// InFlightHTLCs returns all HTLCs that are in flight.
 	InFlightHTLCs() []channeldb.HTLCAttempt
 
-	// GetFailureReason returns the reason the payment failed.
-	GetFailureReason() *channeldb.FailureReason
+	// AllowMoreAttempts is used to decide whether we can safely attempt
+	// more HTLCs for a given payment state. Return an error if the payment
+	// is in an unexpected state.
+	AllowMoreAttempts() (bool, error)
+
+	// TerminalInfo returns the settled HTLC attempt or the payment's
+	// failure reason.
+	TerminalInfo() (*channeldb.HTLCAttempt, *channeldb.FailureReason)
 }
 
 // ControlTower tracks all outgoing payments made, whose primary purpose is to

routing/control_tower_test.go

@@ -134,8 +134,8 @@ func TestControlTowerSubscribeSuccess(t *testing.T) {
 			"subscriber %v failed, want %s, got %s", i,
 			channeldb.StatusSucceeded, result.GetStatus())
 
-		settle, _ := result.TerminalInfo()
-		if settle.Preimage != preimg {
+		attempt, _ := result.TerminalInfo()
+		if attempt.Settle.Preimage != preimg {
 			t.Fatal("unexpected preimage")
 		}
 		if len(result.HTLCs) != 1 {
@@ -264,9 +264,8 @@ func TestPaymentControlSubscribeAllSuccess(t *testing.T) {
 	)
 
 	settle1, _ := result1.TerminalInfo()
-	require.Equal(
-		t, preimg1, settle1.Preimage, "unexpected preimage payment 1",
-	)
+	require.Equal(t, preimg1, settle1.Settle.Preimage,
+		"unexpected preimage payment 1")
 
 	require.Len(
 		t, result1.HTLCs, 1, "expect 1 htlc for payment 1, got %d",
@@ -283,9 +282,8 @@ func TestPaymentControlSubscribeAllSuccess(t *testing.T) {
 	)
 
 	settle2, _ := result2.TerminalInfo()
-	require.Equal(
-		t, preimg2, settle2.Preimage, "unexpected preimage payment 2",
-	)
+	require.Equal(t, preimg2, settle2.Settle.Preimage,
+		"unexpected preimage payment 2")
 	require.Len(
 		t, result2.HTLCs, 1, "expect 1 htlc for payment 2, got %d",
 		len(result2.HTLCs),

routing/mock_test.go

@@ -12,7 +12,9 @@ import (
 	"github.com/lightningnetwork/lnd/htlcswitch"
 	"github.com/lightningnetwork/lnd/lntypes"
 	"github.com/lightningnetwork/lnd/lnwire"
+	"github.com/lightningnetwork/lnd/record"
 	"github.com/lightningnetwork/lnd/routing/route"
+	"github.com/lightningnetwork/lnd/routing/shards"
 	"github.com/stretchr/testify/mock"
 )
 
@@ -572,8 +574,6 @@ func (m *mockControlTowerOld) SubscribeAllPayments() (
 
 type mockPaymentAttemptDispatcher struct {
 	mock.Mock
-
-	resultChan chan *htlcswitch.PaymentResult
 }
 
 var _ PaymentAttemptDispatcher = (*mockPaymentAttemptDispatcher)(nil)
@@ -589,11 +589,14 @@ func (m *mockPaymentAttemptDispatcher) GetAttemptResult(attemptID uint64,
 	paymentHash lntypes.Hash, deobfuscator htlcswitch.ErrorDecrypter) (
 	<-chan *htlcswitch.PaymentResult, error) {
 
-	m.Called(attemptID, paymentHash, deobfuscator)
+	args := m.Called(attemptID, paymentHash, deobfuscator)
 
-	// Instead of returning the mocked returned values, we need to return
-	// the chan resultChan so it can be converted into a read-only chan.
-	return m.resultChan, nil
+	resultChan := args.Get(0)
+	if resultChan == nil {
+		return nil, args.Error(1)
+	}
+
+	return args.Get(0).(chan *htlcswitch.PaymentResult), args.Error(1)
 }
 
 func (m *mockPaymentAttemptDispatcher) CleanStore(
@@ -673,6 +676,12 @@ func (m *mockPaymentSession) RequestRoute(maxAmt, feeLimit lnwire.MilliSatoshi,
 	activeShards, height uint32) (*route.Route, error) {
 
 	args := m.Called(maxAmt, feeLimit, activeShards, height)
+
+	// Type assertion on nil will fail, so we check and return here.
+	if args.Get(0) == nil {
+		return nil, args.Error(1)
+	}
+
 	return args.Get(0).(*route.Route), args.Error(1)
 }
 
@@ -692,7 +701,6 @@ func (m *mockPaymentSession) GetAdditionalEdgePolicy(pubKey *btcec.PublicKey,
 
 type mockControlTower struct {
 	mock.Mock
-	sync.Mutex
 }
 
 var _ ControlTower = (*mockControlTower)(nil)
@@ -712,9 +720,6 @@ func (m *mockControlTower) DeleteFailedAttempts(phash lntypes.Hash) error {
 func (m *mockControlTower) RegisterAttempt(phash lntypes.Hash,
 	a *channeldb.HTLCAttemptInfo) error {
 
-	m.Lock()
-	defer m.Unlock()
-
 	args := m.Called(phash, a)
 	return args.Error(0)
 }
@@ -723,29 +728,32 @@ func (m *mockControlTower) SettleAttempt(phash lntypes.Hash,
 	pid uint64, settleInfo *channeldb.HTLCSettleInfo) (
 	*channeldb.HTLCAttempt, error) {
 
-	m.Lock()
-	defer m.Unlock()
-
 	args := m.Called(phash, pid, settleInfo)
-	return args.Get(0).(*channeldb.HTLCAttempt), args.Error(1)
+
+	attempt := args.Get(0)
+	if attempt == nil {
+		return nil, args.Error(1)
+	}
+
+	return attempt.(*channeldb.HTLCAttempt), args.Error(1)
 }
 
 func (m *mockControlTower) FailAttempt(phash lntypes.Hash, pid uint64,
 	failInfo *channeldb.HTLCFailInfo) (*channeldb.HTLCAttempt, error) {
 
-	m.Lock()
-	defer m.Unlock()
-
 	args := m.Called(phash, pid, failInfo)
+
+	attempt := args.Get(0)
+	if attempt == nil {
+		return nil, args.Error(1)
+	}
+
 	return args.Get(0).(*channeldb.HTLCAttempt), args.Error(1)
 }
 
 func (m *mockControlTower) FailPayment(phash lntypes.Hash,
 	reason channeldb.FailureReason) error {
 
-	m.Lock()
-	defer m.Unlock()
-
 	args := m.Called(phash, reason)
 	return args.Error(0)
 }
@@ -822,15 +830,32 @@ func (m *mockMPPayment) InFlightHTLCs() []channeldb.HTLCAttempt {
 	return args.Get(0).([]channeldb.HTLCAttempt)
 }
 
-func (m *mockMPPayment) GetFailureReason() *channeldb.FailureReason {
+func (m *mockMPPayment) AllowMoreAttempts() (bool, error) {
+	args := m.Called()
+	return args.Bool(0), args.Error(1)
+}
+
+func (m *mockMPPayment) TerminalInfo() (*channeldb.HTLCAttempt,
+	*channeldb.FailureReason) {
+
 	args := m.Called()
 
-	reason := args.Get(0)
-	if reason == nil {
-		return nil
+	var (
+		settleInfo  *channeldb.HTLCAttempt
+		failureInfo *channeldb.FailureReason
+	)
+
+	settle := args.Get(0)
+	if settle != nil {
+		settleInfo = settle.(*channeldb.HTLCAttempt)
 	}
 
-	return reason.(*channeldb.FailureReason)
+	reason := args.Get(1)
+	if reason != nil {
+		failureInfo = reason.(*channeldb.FailureReason)
+	}
+
+	return settleInfo, failureInfo
 }
 
 type mockLink struct {
@@ -854,3 +879,70 @@ func (m *mockLink) EligibleToForward() bool {
 func (m *mockLink) MayAddOutgoingHtlc(_ lnwire.MilliSatoshi) error {
 	return m.mayAddOutgoingErr
 }
+
+type mockShardTracker struct {
+	mock.Mock
+}
+
+var _ shards.ShardTracker = (*mockShardTracker)(nil)
+
+func (m *mockShardTracker) NewShard(attemptID uint64,
+	lastShard bool) (shards.PaymentShard, error) {
+
+	args := m.Called(attemptID, lastShard)
+
+	shard := args.Get(0)
+	if shard == nil {
+		return nil, args.Error(1)
+	}
+
+	return shard.(shards.PaymentShard), args.Error(1)
+}
+
+func (m *mockShardTracker) GetHash(attemptID uint64) (lntypes.Hash, error) {
+	args := m.Called(attemptID)
+	return args.Get(0).(lntypes.Hash), args.Error(1)
+}
+
+func (m *mockShardTracker) CancelShard(attemptID uint64) error {
+	args := m.Called(attemptID)
+	return args.Error(0)
+}
+
+type mockShard struct {
+	mock.Mock
+}
+
+var _ shards.PaymentShard = (*mockShard)(nil)
+
+// Hash returns the hash used for the HTLC representing this shard.
+func (m *mockShard) Hash() lntypes.Hash {
+	args := m.Called()
+	return args.Get(0).(lntypes.Hash)
+}
+
+// MPP returns any extra MPP records that should be set for the final
+// hop on the route used by this shard.
+func (m *mockShard) MPP() *record.MPP {
+	args := m.Called()
+
+	r := args.Get(0)
+	if r == nil {
+		return nil
+	}
+
+	return r.(*record.MPP)
+}
+
+// AMP returns any extra AMP records that should be set for the final
+// hop on the route used by this shard.
+func (m *mockShard) AMP() *record.AMP {
+	args := m.Called()
+
+	r := args.Get(0)
+	if r == nil {
+		return nil
+	}
+
+	return r.(*record.AMP)
+}

routing/router.go

@@ -2,7 +2,6 @@ package routing
 
 import (
 	"bytes"
-	goErrors "errors"
 	"fmt"
 	"math"
 	"runtime"
@@ -120,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
@@ -2451,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
@@ -2496,88 +2506,92 @@ func (r *ChannelRouter) sendToRoute(htlcHash lntypes.Hash, rt *route.Route,
 	// shard we'll now launch.
 	shardTracker := shards.NewSimpleShardTracker(htlcHash, nil)
 
-	// Launch a shard along the given route.
-	sh := &shardHandler{
-		router:       r,
-		identifier:   paymentIdentifier,
-		shardTracker: shardTracker,
-	}
+	// Create a payment lifecycle using the given route with,
+	// - zero fee limit as we are not requesting routes.
+	// - nil payment session (since we already have a route).
+	// - no payment timeout.
+	// - no current block height.
+	p := newPaymentLifecycle(
+		r, 0, paymentIdentifier, nil, shardTracker, 0, 0,
+	)
 
-	var shardError error
-	attempt, outcome, err := sh.launchShard(rt, false)
-
-	// With SendToRoute, it can happen that the route exceeds protocol
-	// constraints. Mark the payment as failed with an internal error.
-	if err == route.ErrMaxRouteHopsExceeded ||
-		err == sphinx.ErrMaxRoutingInfoSizeExceeded {
-
-		log.Debugf("Invalid route provided for payment %x: %v",
-			paymentIdentifier, err)
-
-		controlErr := r.cfg.Control.FailPayment(
-			paymentIdentifier, channeldb.FailureReasonError,
-		)
-		if controlErr != nil {
-			return nil, controlErr
-		}
-	}
-
-	// In any case, don't continue if there is an error.
+	// We found a route to try, create a new HTLC attempt to try.
+	//
+	// NOTE: we use zero `remainingAmt` here to simulate the same effect of
+	// setting the lastShard to be false, which is used by previous
+	// implementation.
+	attempt, err := p.registerAttempt(rt, 0)
 	if err != nil {
 		return nil, err
 	}
 
-	var htlcAttempt *channeldb.HTLCAttempt
-	switch {
-	// Failed to launch shard.
-	case outcome.err != nil:
-		shardError = outcome.err
-		htlcAttempt = outcome.attempt
+	// Once the attempt is created, send it to the htlcswitch. Notice that
+	// the `err` returned here has already been processed by
+	// `handleSwitchErr`, which means if there's a terminal failure, the
+	// payment has been failed.
+	result, err := p.sendAttempt(attempt)
+	if err != nil {
+		return nil, err
+	}
 
-	// Shard successfully launched, wait for the result to be available.
-	default:
-		result, err := sh.collectResult(attempt)
+	// We now lookup the payment to see if it's already failed.
+	payment, err := p.router.cfg.Control.FetchPayment(p.identifier)
+	if err != nil {
+		return result.attempt, err
+	}
+
+	// Exit if the above error has caused the payment to be failed, we also
+	// return the error from sending attempt to mimic the old behavior of
+	// this method.
+	_, failedReason := payment.TerminalInfo()
+	if failedReason != nil {
+		return result.attempt, result.err
+	}
+
+	// Since for SendToRoute we won't retry in case the shard fails, we'll
+	// mark the payment failed with the control tower immediately if the
+	// skipTempErr is false.
+	reason := channeldb.FailureReasonError
+
+	// If we failed to send the HTLC, we need to further decide if we want
+	// to fail the payment.
+	if result.err != nil {
+		// If skipTempErr, we'll return the attempt and the temp error.
+		if skipTempErr {
+			return result.attempt, result.err
+		}
+
+		// Otherwise we need to fail the payment.
+		err := r.cfg.Control.FailPayment(paymentIdentifier, reason)
 		if err != nil {
 			return nil, err
 		}
 
-		// We got a successful result.
-		if result.err == nil {
-			return result.attempt, nil
-		}
-
-		// The shard failed, break switch to handle it.
-		shardError = result.err
-		htlcAttempt = result.attempt
+		return result.attempt, result.err
 	}
 
-	// Since for SendToRoute we won't retry in case the shard fails, we'll
-	// mark the payment failed with the control tower immediately. Process
-	// the error to check if it maps into a terminal error code, if not use
-	// a generic NO_ROUTE error.
-	var failureReason *channeldb.FailureReason
-	err = sh.handleSwitchErr(attempt, shardError)
-
-	switch {
-	// If a non-terminal error is returned and `skipTempErr` is false, then
-	// we'll use the normal no route error.
-	case err == nil && !skipTempErr:
-		err = r.cfg.Control.FailPayment(
-			paymentIdentifier, channeldb.FailureReasonNoRoute,
-		)
-
-	// If this is a failure reason, then we'll apply the failure directly
-	// to the control tower, and return the normal response to the caller.
-	case goErrors.As(err, &failureReason):
-		err = r.cfg.Control.FailPayment(
-			paymentIdentifier, *failureReason,
-		)
-	}
+	// The attempt was successfully sent, wait for the result to be
+	// available.
+	result, err = p.collectResult(attempt)
 	if err != nil {
 		return nil, err
 	}
 
-	return htlcAttempt, shardError
+	// We got a successful result.
+	if result.err == nil {
+		return result.attempt, nil
+	}
+
+	// An error returned from collecting the result, we'll mark the payment
+	// as failed if we don't skip temp error.
+	if !skipTempErr {
+		err := r.cfg.Control.FailPayment(paymentIdentifier, reason)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	return result.attempt, result.err
 }
 
 // sendPayment attempts to send a payment to the passed payment hash. This