Repository: lightningnetwork/lnd
Pull request #7799 (2023-06-30 12:36:08)

routing: fix `index out of range` panic
Make sure that we do not crash the node trying to access an invalid position for an array (-1) in `BuildRoute`.

Fixes https://github.com/lightningnetwork/lnd/issues/7798

lightninglabs-deploy: @positiveblue, remember to re-request review from reviewers when ready
lightninglabs-deploy: @positiveblue, remember to re-request review from reviewers when ready
lightninglabs-deploy: @positiveblue, remember to re-request review from reviewers when ready

Patch:
diff --git a/routing/router.go b/routing/router.go
index 123d13c94..a38980bba 100644
--- a/routing/router.go
+++ b/routing/router.go
@@ -2743,6 +2743,19 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi,
 	// amount that this route can carry.
 	useMinAmt := amt == nil
 
+	var runningAmt lnwire.MilliSatoshi
+	if useMinAmt {
+		// For minimum amount routes, aim to deliver at least 1 msat to
+		// the destination. There are nodes in the wild that have a
+		// min_htlc channel policy of zero, which could lead to a zero
+		// amount payment being made.
+		runningAmt = 1
+	} else {
+		// If an amount is specified, we need to build a route that
+		// delivers exactly this amount to the final destination.
+		runningAmt = *amt
+	}
+
 	// We'll attempt to obtain a set of bandwidth hints that helps us select
 	// the best outgoing channel to use in case no outgoing channel is set.
 	bandwidthHints, err := newBandwidthManager(
@@ -2759,24 +2772,46 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi,
 		return nil, err
 	}
 
-	// Allocate a list that will contain the edge unifiers for this route.
-	unifiers := make([]*edgeUnifier, len(hops))
+	sourceNode := r.selfNode.PubKeyBytes
+	unifiers, senderAmt, err := getRouteUnifiers(
+		sourceNode, hops, useMinAmt, runningAmt, outgoingChans,
+		r.cachedGraph, bandwidthHints,
+	)
+	if err != nil {
+		return nil, err
+	}
 
-	var runningAmt lnwire.MilliSatoshi
-	if useMinAmt {
-		// For minimum amount routes, aim to deliver at least 1 msat to
-		// the destination. There are nodes in the wild that have a
-		// min_htlc channel policy of zero, which could lead to a zero
-		// amount payment being made.
-		runningAmt = 1
-	} else {
-		// If an amount is specified, we need to build a route that
-		// delivers exactly this amount to the final destination.
-		runningAmt = *amt
+	pathEdges, receiverAmt, err := getPathEdges(
+		sourceNode, senderAmt, unifiers, bandwidthHints, hops,
+	)
+	if err != nil {
+		return nil, err
 	}
 
+	// Build and return the final route.
+	return newRoute(
+		sourceNode, pathEdges, uint32(height),
+		finalHopParams{
+			amt:         receiverAmt,
+			totalAmt:    receiverAmt,
+			cltvDelta:   uint16(finalCltvDelta),
+			records:     nil,
+			paymentAddr: payAddr,
+		},
+	)
+}
+
+// getRouteUnifiers returns a list of edge unifiers for the given route.
+func getRouteUnifiers(source route.Vertex, hops []route.Vertex,
+	useMinAmt bool, runningAmt lnwire.MilliSatoshi,
+	outgoingChans map[uint64]struct{}, graph routingGraph,
+	bandwidthHints *bandwidthManager) ([]*edgeUnifier, lnwire.MilliSatoshi,
+	error) {
+
+	// Allocate a list that will contain the edge unifiers for this route.
+	unifiers := make([]*edgeUnifier, len(hops))
+
 	// Traverse hops backwards to accumulate fees in the running amounts.
-	source := r.selfNode.PubKeyBytes
 	for i := len(hops) - 1; i >= 0; i-- {
 		toNode := hops[i]
 
@@ -2793,16 +2828,16 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi,
 		// in the graph.
 		u := newNodeEdgeUnifier(source, toNode, outgoingChans)
 
-		err := u.addGraphPolicies(r.cachedGraph)
+		err := u.addGraphPolicies(graph)
 		if err != nil {
-			return nil, err
+			return nil, 0, err
 		}
 
 		// Exit if there are no channels.
 		edgeUnifier, ok := u.edgeUnifiers[fromNode]
 		if !ok {
 			log.Errorf("Cannot find policy for node %v", fromNode)
-			return nil, ErrNoChannel{
+			return nil, 0, ErrNoChannel{
 				fromNode: fromNode,
 				position: i,
 			}
@@ -2822,7 +2857,7 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi,
 			log.Errorf("Cannot find policy with amt=%v for node %v",
 				runningAmt, fromNode)
 
-			return nil, ErrNoChannel{
+			return nil, 0, ErrNoChannel{
 				fromNode: fromNode,
 				position: i,
 			}
@@ -2839,17 +2874,31 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi,
 		unifiers[i] = edgeUnifier
 	}
 
+	return unifiers, runningAmt, nil
+}
+
+// getPathEdges returns the edges that make up the path and the total amount,
+// including fees, to send the payment.
+func getPathEdges(source route.Vertex, receiverAmt lnwire.MilliSatoshi,
+	unifiers []*edgeUnifier, bandwidthHints *bandwidthManager,
+	hops []route.Vertex) ([]*channeldb.CachedEdgePolicy,
+	lnwire.MilliSatoshi, error) {
+
 	// Now that we arrived at the start of the route and found out the route
 	// total amount, we make a forward pass. Because the amount may have
 	// been increased in the backward pass, fees need to be recalculated and
 	// amount ranges re-checked.
 	var pathEdges []*channeldb.CachedEdgePolicy
-	receiverAmt := runningAmt
 	for i, unifier := range unifiers {
 		edge := unifier.getEdge(receiverAmt, bandwidthHints)
 		if edge == nil {
-			return nil, ErrNoChannel{
-				fromNode: hops[i-1],
+			fromNode := source
+			if i > 0 {
+				fromNode = hops[i-1]
+			}
+
+			return nil, 0, ErrNoChannel{
+				fromNode: fromNode,
 				position: i,
 			}
 		}
@@ -2864,15 +2913,5 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi,
 		pathEdges = append(pathEdges, edge.policy)
 	}
 
-	// Build and return the final route.
-	return newRoute(
-		source, pathEdges, uint32(height),
-		finalHopParams{
-			amt:         receiverAmt,
-			totalAmt:    receiverAmt,
-			cltvDelta:   uint16(finalCltvDelta),
-			records:     nil,
-			paymentAddr: payAddr,
-		},
-	)
+	return pathEdges, receiverAmt, nil
 }


Test patch:
diff --git a/invoices/invoiceregistry_test.go b/invoices/invoiceregistry_test.go
index 1b93b8f24..1edd753cd 100644
--- a/invoices/invoiceregistry_test.go
+++ b/invoices/invoiceregistry_test.go
@@ -19,8 +19,6 @@ import (
 
 // TestSettleInvoice tests settling of an invoice and related notifications.
 func TestSettleInvoice(t *testing.T) {
-	t.Parallel()
-
 	ctx := newTestContext(t, nil)
 
 	allSubscriptions, err := ctx.registry.SubscribeNotifications(0, 0)
@@ -39,7 +37,6 @@ func TestSettleInvoice(t *testing.T) {
 	require.Equal(t, subscription.PayHash(), &testInvoicePaymentHash)
 
 	// Add the invoice.
-	testInvoice := newInvoice(t, false)
 	addIdx, err := ctx.registry.AddInvoice(
 		testInvoice, testInvoicePaymentHash,
 	)
@@ -196,8 +193,6 @@ func TestSettleInvoice(t *testing.T) {
 }
 
 func testCancelInvoice(t *testing.T, gc bool) {
-	t.Parallel()
-
 	cfg := defaultRegistryConfig()
 
 	// If set to true, then also delete the invoice from the DB after
@@ -225,7 +220,7 @@ func testCancelInvoice(t *testing.T, gc bool) {
 	require.Equal(t, subscription.PayHash(), &testInvoicePaymentHash)
 
 	// Add the invoice.
-	testInvoice := newInvoice(t, false)
+	amt := lnwire.MilliSatoshi(100000)
 	_, err = ctx.registry.AddInvoice(testInvoice, testInvoicePaymentHash)
 	if err != nil {
 		t.Fatal(err)
@@ -303,8 +298,8 @@ func testCancelInvoice(t *testing.T, gc bool) {
 	// result in a cancel resolution.
 	hodlChan := make(chan interface{})
 	resolution, err := ctx.registry.NotifyExitHopHtlc(
-		testInvoicePaymentHash, testInvoiceAmount, testHtlcExpiry,
-		testCurrentHeight, getCircuitKey(0), hodlChan, testPayload,
+		testInvoicePaymentHash, amt, testHtlcExpiry, testCurrentHeight,
+		getCircuitKey(0), hodlChan, testPayload,
 	)
 	if err != nil {
 		t.Fatal("expected settlement of a canceled invoice to succeed")
@@ -330,8 +325,6 @@ func testCancelInvoice(t *testing.T, gc bool) {
 
 // TestCancelInvoice tests cancellation of an invoice and related notifications.
 func TestCancelInvoice(t *testing.T) {
-	t.Parallel()
-
 	// Test cancellation both with garbage collection (meaning that canceled
 	// invoice will be deleted) and without (meaning it'll be kept).
 	t.Run("garbage collect", func(t *testing.T) {
@@ -346,7 +339,6 @@ func TestCancelInvoice(t *testing.T) {
 // TestSettleHoldInvoice tests settling of a hold invoice and related
 // notifications.
 func TestSettleHoldInvoice(t *testing.T) {
-	t.Parallel()
 	defer timeout()()
 
 	idb, err := newTestChannelDB(t, clock.NewTestClock(time.Time{}))
@@ -387,8 +379,7 @@ func TestSettleHoldInvoice(t *testing.T) {
 	require.Equal(t, subscription.PayHash(), &testInvoicePaymentHash)
 
 	// Add the invoice.
-	invoice := newInvoice(t, true)
-	_, err = registry.AddInvoice(invoice, testInvoicePaymentHash)
+	_, err = registry.AddInvoice(testHodlInvoice, testInvoicePaymentHash)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -520,7 +511,6 @@ func TestSettleHoldInvoice(t *testing.T) {
 // TestCancelHoldInvoice tests canceling of a hold invoice and related
 // notifications.
 func TestCancelHoldInvoice(t *testing.T) {
-	t.Parallel()
 	defer timeout()()
 
 	testClock := clock.NewTestClock(testTime)
@@ -546,8 +536,7 @@ func TestCancelHoldInvoice(t *testing.T) {
 	})
 
 	// Add the invoice.
-	invoice := newInvoice(t, true)
-	_, err = registry.AddInvoice(invoice, testInvoicePaymentHash)
+	_, err = registry.AddInvoice(testHodlInvoice, testInvoicePaymentHash)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -601,7 +590,6 @@ func TestCancelHoldInvoice(t *testing.T) {
 // if we are the exit hop, but in htlcIncomingContestResolver it is called with
 // forwarded htlc hashes as well.
 func TestUnknownInvoice(t *testing.T) {
-	t.Parallel()
 	ctx := newTestContext(t, nil)
 
 	// Notify arrival of a new htlc paying to this invoice. This should
@@ -622,8 +610,6 @@ func TestUnknownInvoice(t *testing.T) {
 // TestKeySend tests receiving a spontaneous payment with and without keysend
 // enabled.
 func TestKeySend(t *testing.T) {
-	t.Parallel()
-
 	t.Run("enabled", func(t *testing.T) {
 		testKeySend(t, true)
 	})
@@ -635,7 +621,6 @@ func TestKeySend(t *testing.T) {
 // testKeySend is the inner test function that tests keysend for a particular
 // enabled state on the receiver end.
 func testKeySend(t *testing.T, keySendEnabled bool) {
-	t.Parallel()
 	defer timeout()()
 
 	cfg := defaultRegistryConfig()
@@ -750,8 +735,6 @@ func testKeySend(t *testing.T, keySendEnabled bool) {
 
 // TestHoldKeysend tests receiving a spontaneous payment that is held.
 func TestHoldKeysend(t *testing.T) {
-	t.Parallel()
-
 	t.Run("settle", func(t *testing.T) {
 		testHoldKeysend(t, false)
 	})
@@ -762,7 +745,6 @@ func TestHoldKeysend(t *testing.T) {
 
 // testHoldKeysend is the inner test function that tests hold-keysend.
 func testHoldKeysend(t *testing.T, timeoutKeysend bool) {
-	t.Parallel()
 	defer timeout()()
 
 	const holdDuration = time.Minute
@@ -853,20 +835,18 @@ func testHoldKeysend(t *testing.T, timeoutKeysend bool) {
 // It covers the case where there is a mpp timeout before the whole invoice is
 // paid and the case where the invoice is settled in time.
 func TestMppPayment(t *testing.T) {
-	t.Parallel()
 	defer timeout()()
 
 	ctx := newTestContext(t, nil)
 
 	// Add the invoice.
-	testInvoice := newInvoice(t, false)
 	_, err := ctx.registry.AddInvoice(testInvoice, testInvoicePaymentHash)
 	if err != nil {
 		t.Fatal(err)
 	}
 
 	mppPayload := &mockPayload{
-		mpp: record.NewMPP(testInvoiceAmount, [32]byte{}),
+		mpp: record.NewMPP(testInvoiceAmt, [32]byte{}),
 	}
 
 	// Send htlc 1.
@@ -956,16 +936,16 @@ func TestMppPaymentWithOverpayment(t *testing.T) {
 		ctx := newTestContext(t, nil)
 
 		// Add the invoice.
-		testInvoice := newInvoice(t, false)
+		invoice := *testInvoice
 		_, err := ctx.registry.AddInvoice(
-			testInvoice, testInvoicePaymentHash,
+			&invoice, testInvoicePaymentHash,
 		)
 		if err != nil {
 			t.Fatal(err)
 		}
 
 		mppPayload := &mockPayload{
-			mpp: record.NewMPP(testInvoiceAmount, [32]byte{}),
+			mpp: record.NewMPP(testInvoiceAmt, [32]byte{}),
 		}
 
 		// We constrain overpayment amount to be [1,1000].
@@ -974,7 +954,7 @@ func TestMppPaymentWithOverpayment(t *testing.T) {
 		// Send htlc 1.
 		hodlChan1 := make(chan interface{}, 1)
 		resolution, err := ctx.registry.NotifyExitHopHtlc(
-			testInvoicePaymentHash, testInvoice.Terms.Value/2,
+			testInvoicePaymentHash, invoice.Terms.Value/2,
 			testHtlcExpiry, testCurrentHeight, getCircuitKey(11),
 			hodlChan1, mppPayload,
 		)
@@ -989,7 +969,7 @@ func TestMppPaymentWithOverpayment(t *testing.T) {
 		hodlChan2 := make(chan interface{}, 1)
 		resolution, err = ctx.registry.NotifyExitHopHtlc(
 			testInvoicePaymentHash,
-			testInvoice.Terms.Value/2+overpayment, testHtlcExpiry,
+			invoice.Terms.Value/2+overpayment, testHtlcExpiry,
 			testCurrentHeight, getCircuitKey(12), hodlChan2,
 			mppPayload,
 		)
@@ -1017,7 +997,7 @@ func TestMppPaymentWithOverpayment(t *testing.T) {
 			t.Fatal("expected invoice to be settled")
 		}
 
-		return inv.AmtPaid == testInvoice.Terms.Value+overpayment
+		return inv.AmtPaid == invoice.Terms.Value+overpayment
 	}
 	if err := quick.Check(f, nil); err != nil {
 		t.Fatalf("amount incorrect: %v", err)
@@ -1215,8 +1195,6 @@ func TestOldInvoiceRemovalOnStart(t *testing.T) {
 // invoice is settled before expiry (and thus not canceled), and the case
 // where the invoice is expired.
 func TestHeightExpiryWithRegistry(t *testing.T) {
-	t.Parallel()
-
 	t.Run("single shot settled before expiry", func(t *testing.T) {
 		testHeightExpiryWithRegistry(t, 1, true)
 	})
@@ -1244,21 +1222,21 @@ func testHeightExpiryWithRegistry(t *testing.T, numParts int, settle bool) {
 
 	// Add a hold invoice, we set a non-nil payment request so that this
 	// invoice is not considered a keysend by the expiry watcher.
-	testInvoice := newInvoice(t, false)
-	testInvoice.HodlInvoice = true
-	testInvoice.PaymentRequest = []byte{1, 2, 3}
+	invoice := *testInvoice
+	invoice.HodlInvoice = true
+	invoice.PaymentRequest = []byte{1, 2, 3}
 
-	_, err := ctx.registry.AddInvoice(testInvoice, testInvoicePaymentHash)
+	_, err := ctx.registry.AddInvoice(&invoice, testInvoicePaymentHash)
 	require.NoError(t, err)
 
 	payLoad := testPayload
 	if numParts > 1 {
 		payLoad = &mockPayload{
-			mpp: record.NewMPP(testInvoiceAmount, [32]byte{}),
+			mpp: record.NewMPP(testInvoiceAmt, [32]byte{}),
 		}
 	}
 
-	htlcAmt := testInvoice.Terms.Value / lnwire.MilliSatoshi(numParts)
+	htlcAmt := invoice.Terms.Value / lnwire.MilliSatoshi(numParts)
 	hodlChan := make(chan interface{}, numParts)
 	for i := 0; i < numParts; i++ {
 		// We bump our expiry height for each htlc so that we can test
@@ -1284,9 +1262,7 @@ func testHeightExpiryWithRegistry(t *testing.T, numParts int, settle bool) {
 	// Now that we've added our htlc(s), we tick our test clock to our
 	// invoice expiry time. We don't expect the invoice to be canceled
 	// based on its expiry time now that we have active htlcs.
-	ctx.clock.SetTime(
-		testInvoice.CreationDate.Add(testInvoice.Terms.Expiry + 1),
-	)
+	ctx.clock.SetTime(invoice.CreationDate.Add(invoice.Terms.Expiry + 1))
 
 	// The expiry watcher loop takes some time to process the new clock
 	// time. We mine the block before our expiry height, our mock will block
@@ -1350,19 +1326,20 @@ func TestMultipleSetHeightExpiry(t *testing.T) {
 	ctx := newTestContext(t, nil)
 
 	// Add a hold invoice.
-	testInvoice := newInvoice(t, true)
+	invoice := *testInvoice
+	invoice.HodlInvoice = true
 
-	_, err := ctx.registry.AddInvoice(testInvoice, testInvoicePaymentHash)
+	_, err := ctx.registry.AddInvoice(&invoice, testInvoicePaymentHash)
 	require.NoError(t, err)
 
 	mppPayload := &mockPayload{
-		mpp: record.NewMPP(testInvoiceAmount, [32]byte{}),
+		mpp: record.NewMPP(testInvoiceAmt, [32]byte{}),
 	}
 
 	// Send htlc 1.
 	hodlChan1 := make(chan interface{}, 1)
 	resolution, err := ctx.registry.NotifyExitHopHtlc(
-		testInvoicePaymentHash, testInvoice.Terms.Value/2,
+		testInvoicePaymentHash, invoice.Terms.Value/2,
 		testHtlcExpiry,
 		testCurrentHeight, getCircuitKey(10), hodlChan1, mppPayload,
 	)
@@ -1392,7 +1369,7 @@ func TestMultipleSetHeightExpiry(t *testing.T) {
 	// Send htlc 2.
 	hodlChan2 := make(chan interface{}, 1)
 	resolution, err = ctx.registry.NotifyExitHopHtlc(
-		testInvoicePaymentHash, testInvoice.Terms.Value/2, expiry,
+		testInvoicePaymentHash, invoice.Terms.Value/2, expiry,
 		testCurrentHeight, getCircuitKey(11), hodlChan2, mppPayload,
 	)
 	require.NoError(t, err)
@@ -1401,7 +1378,7 @@ func TestMultipleSetHeightExpiry(t *testing.T) {
 	// Send htlc 3.
 	hodlChan3 := make(chan interface{}, 1)
 	resolution, err = ctx.registry.NotifyExitHopHtlc(
-		testInvoicePaymentHash, testInvoice.Terms.Value/2, expiry,
+		testInvoicePaymentHash, invoice.Terms.Value/2, expiry,
 		testCurrentHeight, getCircuitKey(12), hodlChan3, mppPayload,
 	)
 	require.NoError(t, err)
@@ -1449,25 +1426,10 @@ func TestSettleInvoicePaymentAddrRequired(t *testing.T) {
 
 	require.Equal(t, subscription.PayHash(), &testInvoicePaymentHash)
 
-	invoice := &invpkg.Invoice{
-		Terms: invpkg.ContractTerm{
-			PaymentPreimage: &testInvoicePreimage,
-			Value:           testInvoiceAmount,
-			Expiry:          time.Hour,
-			Features: lnwire.NewFeatureVector(
-				lnwire.NewRawFeatureVector(
-					lnwire.TLVOnionPayloadOptional,
-					lnwire.PaymentAddrRequired,
-				),
-				lnwire.Features,
-			),
-		},
-		CreationDate: testInvoiceCreationDate,
-	}
 	// Add the invoice, which requires the MPP payload to always be
 	// included due to its set of feature bits.
 	addIdx, err := ctx.registry.AddInvoice(
-		invoice, testInvoicePaymentHash,
+		testPayAddrReqInvoice, testInvoicePaymentHash,
 	)
 	require.NoError(t, err)
 	require.Equal(t, int(addIdx), 1)
@@ -1500,7 +1462,7 @@ func TestSettleInvoicePaymentAddrRequired(t *testing.T) {
 	// information, so it should be forced to the updateLegacy path then
 	// fail as a required feature bit exists.
 	resolution, err := ctx.registry.NotifyExitHopHtlc(
-		testInvoicePaymentHash, invoice.Terms.Value,
+		testInvoicePaymentHash, testInvoice.Terms.Value,
 		uint32(testCurrentHeight)+testInvoiceCltvDelta-1,
 		testCurrentHeight, getCircuitKey(10), hodlChan, testPayload,
 	)
@@ -1537,26 +1499,10 @@ func TestSettleInvoicePaymentAddrRequiredOptionalGrace(t *testing.T) {
 
 	require.Equal(t, subscription.PayHash(), &testInvoicePaymentHash)
 
-	invoice := &invpkg.Invoice{
-		Terms: invpkg.ContractTerm{
-			PaymentPreimage: &testInvoicePreimage,
-			Value:           testInvoiceAmount,
-			Expiry:          time.Hour,
-			Features: lnwire.NewFeatureVector(
-				lnwire.NewRawFeatureVector(
-					lnwire.TLVOnionPayloadOptional,
-					lnwire.PaymentAddrOptional,
-				),
-				lnwire.Features,
-			),
-		},
-		CreationDate: testInvoiceCreationDate,
-	}
-
-	// Add the invoice, which does not require the MPP payload to always be
+	// Add the invoice, which requires the MPP payload to always be
 	// included due to its set of feature bits.
 	addIdx, err := ctx.registry.AddInvoice(
-		invoice, testInvoicePaymentHash,
+		testPayAddrOptionalInvoice, testInvoicePaymentHash,
 	)
 	require.NoError(t, err)
 	require.Equal(t, int(addIdx), 1)
@@ -1588,7 +1534,7 @@ func TestSettleInvoicePaymentAddrRequiredOptionalGrace(t *testing.T) {
 	// no problem as we should allow these existing invoices to be settled.
 	hodlChan := make(chan interface{}, 1)
 	resolution, err := ctx.registry.NotifyExitHopHtlc(
-		testInvoicePaymentHash, testInvoiceAmount,
+		testInvoicePaymentHash, testInvoiceAmt,
 		testHtlcExpiry, testCurrentHeight,
 		getCircuitKey(10), hodlChan, testPayload,
 	)
@@ -1609,7 +1555,7 @@ func TestSettleInvoicePaymentAddrRequiredOptionalGrace(t *testing.T) {
 			t.Fatalf("expected state ContractOpen, but got %v",
 				update.State)
 		}
-		if update.AmtPaid != invoice.Terms.Value {
+		if update.AmtPaid != testInvoice.Terms.Value {
 			t.Fatal("invoice AmtPaid incorrect")
 		}
 	case <-time.After(testTimeout):
@@ -1631,7 +1577,6 @@ func TestSettleInvoicePaymentAddrRequiredOptionalGrace(t *testing.T) {
 // TestAMPWithoutMPPPayload asserts that we correctly reject an AMP HTLC that
 // does not include an MPP record.
 func TestAMPWithoutMPPPayload(t *testing.T) {
-	t.Parallel()
 	defer timeout()()
 
 	cfg := defaultRegistryConfig()
@@ -1664,8 +1609,6 @@ func TestAMPWithoutMPPPayload(t *testing.T) {
 // TestSpontaneousAmpPayment tests receiving a spontaneous AMP payment with both
 // valid and invalid reconstructions.
 func TestSpontaneousAmpPayment(t *testing.T) {
-	t.Parallel()
-
 	tests := []struct {
 		name               string
 		ampEnabled         bool
@@ -1719,7 +1662,6 @@ func TestSpontaneousAmpPayment(t *testing.T) {
 func testSpontaneousAmpPayment(
 	t *testing.T, ampEnabled, failReconstruction bool, numShards int) {
 
-	t.Parallel()
 	defer timeout()()
 
 	cfg := defaultRegistryConfig()
diff --git a/invoices/test_utils_test.go b/invoices/test_utils_test.go
index 13cd11184..e16b3ecc3 100644
--- a/invoices/test_utils_test.go
+++ b/invoices/test_utils_test.go
@@ -131,6 +131,61 @@ var (
 	testInvoiceCreationDate = testTime
 )
 
+var (
+	testInvoiceAmt = lnwire.MilliSatoshi(100000)
+	testInvoice    = &invpkg.Invoice{
+		Terms: invpkg.ContractTerm{
+			PaymentPreimage: &testInvoicePreimage,
+			Value:           testInvoiceAmt,
+			Expiry:          time.Hour,
+			Features:        testFeatures,
+		},
+		CreationDate: testInvoiceCreationDate,
+	}
+
+	testPayAddrReqInvoice = &invpkg.Invoice{
+		Terms: invpkg.ContractTerm{
+			PaymentPreimage: &testInvoicePreimage,
+			Value:           testInvoiceAmt,
+			Expiry:          time.Hour,
+			Features: lnwire.NewFeatureVector(
+				lnwire.NewRawFeatureVector(
+					lnwire.TLVOnionPayloadOptional,
+					lnwire.PaymentAddrRequired,
+				),
+				lnwire.Features,
+			),
+		},
+		CreationDate: testInvoiceCreationDate,
+	}
+
+	testPayAddrOptionalInvoice = &invpkg.Invoice{
+		Terms: invpkg.ContractTerm{
+			PaymentPreimage: &testInvoicePreimage,
+			Value:           testInvoiceAmt,
+			Expiry:          time.Hour,
+			Features: lnwire.NewFeatureVector(
+				lnwire.NewRawFeatureVector(
+					lnwire.TLVOnionPayloadOptional,
+					lnwire.PaymentAddrOptional,
+				),
+				lnwire.Features,
+			),
+		},
+		CreationDate: testInvoiceCreationDate,
+	}
+
+	testHodlInvoice = &invpkg.Invoice{
+		Terms: invpkg.ContractTerm{
+			Value:    testInvoiceAmt,
+			Expiry:   time.Hour,
+			Features: testFeatures,
+		},
+		CreationDate: testInvoiceCreationDate,
+		HodlInvoice:  true,
+	}
+)
+
 func newTestChannelDB(t *testing.T, clock clock.Clock) (*channeldb.DB, error) {
 	t.Helper()
 
@@ -220,35 +275,44 @@ func getCircuitKey(htlcID uint64) invpkg.CircuitKey {
 	}
 }
 
-// newInvoice returns an invoice that can be used for testing, using the
-// constant values defined above (deep copied if necessary).
-//
-// Note that this invoice *does not* have a payment address set. It will
-// create a regular invoice with a preimage is hodl is false, and a hodl
-// invoice with no preimage otherwise.
-func newInvoice(t *testing.T, hodl bool) *invpkg.Invoice {
-	invoice := &invpkg.Invoice{
-		Terms: invpkg.ContractTerm{
-			Value:    testInvoiceAmount,
-			Expiry:   time.Hour,
-			Features: testFeatures.Clone(),
-		},
-		CreationDate: testInvoiceCreationDate,
+func newTestInvoice(t *testing.T, preimage lntypes.Preimage,
+	timestamp time.Time, expiry time.Duration) *invpkg.Invoice {
+
+	if expiry == 0 {
+		expiry = time.Hour
 	}
 
-	// If creating a hodl invoice, we don't include a preimage.
-	if hodl {
-		invoice.HodlInvoice = true
-		return invoice
+	var payAddr [32]byte
+	if _, err := rand.Read(payAddr[:]); err != nil {
+		t.Fatalf("unable to generate payment addr: %v", err)
 	}
 
-	preimage, err := lntypes.MakePreimage(
-		testInvoicePreimage[:],
+	rawInvoice, err := zpay32.NewInvoice(
+		testNetParams,
+		preimage.Hash(),
+		timestamp,
+		zpay32.Amount(testInvoiceAmount),
+		zpay32.Description(testInvoiceDescription),
+		zpay32.Expiry(expiry),
+		zpay32.PaymentAddr(payAddr),
 	)
-	require.NoError(t, err)
-	invoice.Terms.PaymentPreimage = &preimage
+	require.NoError(t, err, "Error while creating new invoice")
+
+	paymentRequest, err := rawInvoice.Encode(testMessageSigner)
 
-	return invoice
+	require.NoError(t, err, "Error while encoding payment request")
+
+	return &invpkg.Invoice{
+		Terms: invpkg.ContractTerm{
+			PaymentPreimage: &preimage,
+			PaymentAddr:     payAddr,
+			Value:           testInvoiceAmount,
+			Expiry:          expiry,
+			Features:        testFeatures,
+		},
+		PaymentRequest: []byte(paymentRequest),
+		CreationDate:   timestamp,
+	}
 }
 
 // timeout implements a test level timeout.
@@ -296,7 +360,7 @@ func generateInvoiceExpiryTestData(
 		var preimage lntypes.Preimage
 		binary.BigEndian.PutUint32(preimage[:4], uint32(offset+i))
 		expiry := time.Duration((i+offset)%24) * time.Hour
-		invoice := newInvoiceExpiryTestInvoice(
+		invoice := newTestInvoice(
 			t, preimage, expiredCreationDate, expiry,
 		)
 		testData.expiredInvoices[preimage.Hash()] = invoice
@@ -306,57 +370,13 @@ func generateInvoiceExpiryTestData(
 		var preimage lntypes.Preimage
 		binary.BigEndian.PutUint32(preimage[4:], uint32(offset+i))
 		expiry := time.Duration((i+offset)%24) * time.Hour
-		invoice := newInvoiceExpiryTestInvoice(t, preimage, now, expiry)
+		invoice := newTestInvoice(t, preimage, now, expiry)
 		testData.pendingInvoices[preimage.Hash()] = invoice
 	}
 
 	return testData
 }
 
-// newInvoiceExpiryTestInvoice creates a test invoice with a randomly generated
-// payment address and custom preimage and expiry details. It should be used in
-// the case where tests require custom invoice expiry and unique payment
-// hashes.
-func newInvoiceExpiryTestInvoice(t *testing.T, preimage lntypes.Preimage,
-	timestamp time.Time, expiry time.Duration) *invpkg.Invoice {
-
-	if expiry == 0 {
-		expiry = time.Hour
-	}
-
-	var payAddr [32]byte
-	if _, err := rand.Read(payAddr[:]); err != nil {
-		t.Fatalf("unable to generate payment addr: %v", err)
-	}
-
-	rawInvoice, err := zpay32.NewInvoice(
-		testNetParams,
-		preimage.Hash(),
-		timestamp,
-		zpay32.Amount(testInvoiceAmount),
-		zpay32.Description(testInvoiceDescription),
-		zpay32.Expiry(expiry),
-		zpay32.PaymentAddr(payAddr),
-	)
-	require.NoError(t, err, "Error while creating new invoice")
-
-	paymentRequest, err := rawInvoice.Encode(testMessageSigner)
-
-	require.NoError(t, err, "Error while encoding payment request")
-
-	return &invpkg.Invoice{
-		Terms: invpkg.ContractTerm{
-			PaymentPreimage: &preimage,
-			PaymentAddr:     payAddr,
-			Value:           testInvoiceAmount,
-			Expiry:          expiry,
-			Features:        testFeatures,
-		},
-		PaymentRequest: []byte(paymentRequest),
-		CreationDate:   timestamp,
-	}
-}
-
 // checkSettleResolution asserts the resolution is a settle with the correct
 // preimage. If successful, the HtlcSettleResolution is returned in case further
 // checks are desired.
diff --git a/routing/router_test.go b/routing/router_test.go
index 767cf7347..27a43d474 100644
--- a/routing/router_test.go
+++ b/routing/router_test.go
@@ -3187,6 +3187,55 @@ func TestBuildRoute(t *testing.T) {
 	}
 }
 
+// TestGetPathEdges tests that the getPathEdges function returns the expected
+// edges and amount when given a set of unifiers and does not panic.
+func TestGetPathEdges(t *testing.T) {
+	t.Parallel()
+
+	const startingBlockHeight = 101
+	ctx := createTestCtxFromFile(t, startingBlockHeight, basicGraphFilePath)
+
+	testCases := []struct {
+		sourceNode     route.Vertex
+		amt            lnwire.MilliSatoshi
+		unifiers       []*edgeUnifier
+		bandwidthHints *bandwidthManager
+		hops           []route.Vertex
+
+		expectedEdges []*channeldb.CachedEdgePolicy
+		expectedAmt   lnwire.MilliSatoshi
+		expectedErr   string
+	}{{
+		sourceNode: ctx.aliases["roasbeef"],
+		unifiers: []*edgeUnifier{
+			{
+				edges:     []*unifiedEdge{},
+				localChan: true,
+			},
+		},
+		expectedErr: fmt.Sprintf("no matching outgoing channel "+
+			"available for node 0 (%v)", ctx.aliases["roasbeef"]),
+	}}
+
+	for _, tc := range testCases {
+		pathEdges, amt, err := getPathEdges(
+			tc.sourceNode, tc.amt, tc.unifiers, tc.bandwidthHints,
+			tc.hops,
+		)
+
+		if tc.expectedErr != "" {
+			require.Error(t, err)
+			require.ErrorContains(t, err, tc.expectedErr)
+
+			continue
+		}
+
+		require.NoError(t, err)
+		require.Equal(t, pathEdges, tc.expectedEdges)
+		require.Equal(t, amt, tc.expectedAmt)
+	}
+}
+
 // edgeCreationModifier is an enum-like type used to modify steps that are
 // skipped when creating a channel in the test context.
 type edgeCreationModifier uint8