routing: assume TLV onion during path finding

routing/additional_edge.go

@@ -25,7 +25,7 @@ type AdditionalEdge interface {
 	// additional edge when being an intermediate hop in a route NOT the
 	// final hop.
 	IntermediatePayloadSize(amount lnwire.MilliSatoshi, expiry uint32,
-		legacy bool, channelID uint64) uint64
+		channelID uint64) uint64
 
 	// EdgePolicy returns the policy of the additional edge.
 	EdgePolicy() *models.CachedEdgePolicy
@@ -33,7 +33,7 @@ type AdditionalEdge interface {
 
 // PayloadSizeFunc defines the interface for the payload size function.
 type PayloadSizeFunc func(amount lnwire.MilliSatoshi, expiry uint32,
-	legacy bool, channelID uint64) uint64
+	channelID uint64) uint64
 
 // PrivateEdge implements the AdditionalEdge interface. As the name implies it
 // is used for private route hints that the receiver adds for example to an
@@ -50,12 +50,11 @@ func (p *PrivateEdge) EdgePolicy() *models.CachedEdgePolicy {
 // IntermediatePayloadSize returns the sphinx payload size defined in BOLT04 if
 // this edge were to be included in a route.
 func (p *PrivateEdge) IntermediatePayloadSize(amount lnwire.MilliSatoshi,
-	expiry uint32, legacy bool, channelID uint64) uint64 {
+	expiry uint32, channelID uint64) uint64 {
 
 	hop := route.Hop{
 		AmtToForward:     amount,
 		OutgoingTimeLock: expiry,
-		LegacyPayload:    legacy,
 	}
 
 	return hop.PayloadSize(channelID)
@@ -77,11 +76,10 @@ func (b *BlindedEdge) EdgePolicy() *models.CachedEdgePolicy {
 // IntermediatePayloadSize returns the sphinx payload size defined in BOLT04 if
 // this edge were to be included in a route.
 func (b *BlindedEdge) IntermediatePayloadSize(_ lnwire.MilliSatoshi, _ uint32,
-	_ bool, _ uint64) uint64 {
+	_ uint64) uint64 {
 
 	hop := route.Hop{
 		BlindingPoint: b.blindingPoint,
-		LegacyPayload: false,
 		EncryptedData: b.cipherText,
 	}
 
@@ -97,11 +95,11 @@ var _ AdditionalEdge = (*BlindedEdge)(nil)
 // defaultHopPayloadSize is the default payload size of a normal (not-blinded)
 // hop in the route.
 func defaultHopPayloadSize(amount lnwire.MilliSatoshi, expiry uint32,
-	legacy bool, channelID uint64) uint64 {
+	channelID uint64) uint64 {
 
 	// The payload size of a cleartext intermediate hop is equal to the
 	// payload size of a private edge therefore we reuse its size function.
 	edge := PrivateEdge{}
 
-	return edge.IntermediatePayloadSize(amount, expiry, legacy, channelID)
+	return edge.IntermediatePayloadSize(amount, expiry, channelID)
 }

routing/additional_edge_test.go

@@ -27,24 +27,12 @@ func TestIntermediatePayloadSize(t *testing.T) {
 		nextHop uint64
 		edge    AdditionalEdge
 	}{
-		{
-			name: "Legacy payload private edge",
-			hop: route.Hop{
-				AmtToForward:     1000,
-				OutgoingTimeLock: 600000,
-				ChannelID:        3432483437438,
-				LegacyPayload:    true,
-			},
-			nextHop: 1,
-			edge:    &PrivateEdge{},
-		},
 		{
 			name: "Tlv payload private edge",
 			hop: route.Hop{
 				AmtToForward:     1000,
 				OutgoingTimeLock: 600000,
 				ChannelID:        3432483437438,
-				LegacyPayload:    false,
 			},
 			nextHop: 1,
 			edge:    &PrivateEdge{},
@@ -86,7 +74,6 @@ func TestIntermediatePayloadSize(t *testing.T) {
 				IntermediatePayloadSize(
 					testCase.hop.AmtToForward,
 					testCase.hop.OutgoingTimeLock,
-					testCase.hop.LegacyPayload,
 					testCase.nextHop,
 				)
 

routing/blinding_test.go

@@ -202,10 +202,10 @@ func TestBlindedPaymentToHints(t *testing.T) {
 		// The arguments we use for the payload do not matter as long as
 		// both functions return the same payload.
 		expectedPayloadSize := expectedHint[0].IntermediatePayloadSize(
-			0, 0, false, 0,
+			0, 0, 0,
 		)
 		actualPayloadSize := actualHint[0].IntermediatePayloadSize(
-			0, 0, false, 0,
+			0, 0, 0,
 		)
 
 		require.Equal(t, expectedPayloadSize, actualPayloadSize)

routing/mocks.go

@@ -1,32 +0,0 @@
-package routing
-
-import (
-	"github.com/lightningnetwork/lnd/channeldb/models"
-	"github.com/lightningnetwork/lnd/lnwire"
-	"github.com/stretchr/testify/mock"
-)
-
-// mockAdditionalEdge is a mock of the AdditionalEdge interface.
-type mockAdditionalEdge struct{ mock.Mock }
-
-// IntermediatePayloadSize returns the sphinx payload size defined in BOLT04 if
-// this edge were to be included in a route.
-func (m *mockAdditionalEdge) IntermediatePayloadSize(amount lnwire.MilliSatoshi,
-	expiry uint32, legacy bool, channelID uint64) uint64 {
-
-	args := m.Called(amount, expiry, legacy, channelID)
-
-	return args.Get(0).(uint64)
-}
-
-// EdgePolicy return the policy of the mockAdditionalEdge.
-func (m *mockAdditionalEdge) EdgePolicy() *models.CachedEdgePolicy {
-	args := m.Called()
-
-	edgePolicy := args.Get(0)
-	if edgePolicy == nil {
-		return nil
-	}
-
-	return edgePolicy.(*models.CachedEdgePolicy)
-}

routing/pathfind.go

@@ -674,8 +674,7 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
 
 	// The payload size of the final hop differ from intermediate hops
 	// and depends on whether the destination is blinded or not.
-	lastHopPayloadSize := lastHopPayloadSize(r, finalHtlcExpiry, amt,
+	lastHopPayloadSize := lastHopPayloadSize(r, finalHtlcExpiry, amt)
-		!features.HasFeature(lnwire.TLVOnionPayloadOptional))
 
 	// We can't always assume that the end destination is publicly
 	// advertised to the network so we'll manually include the target node.
@@ -897,14 +896,10 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
 				return
 			}
 
-			supportsTlv := fromFeatures.HasFeature(
-				lnwire.TLVOnionPayloadOptional,
-			)
-
 			payloadSize = edge.hopPayloadSizeFn(
 				amountToSend,
 				uint32(toNodeDist.incomingCltv),
-				!supportsTlv, edge.policy.ChannelID,
+				edge.policy.ChannelID,
 			)
 		}
 
@@ -1200,7 +1195,7 @@ func getProbabilityBasedDist(weight int64, probability float64,
 // It depends on the tlv types which are present and also whether the hop is
 // part of a blinded route or not.
 func lastHopPayloadSize(r *RestrictParams, finalHtlcExpiry int32,
-	amount lnwire.MilliSatoshi, legacy bool) uint64 {
+	amount lnwire.MilliSatoshi) uint64 {
 
 	if r.BlindedPayment != nil {
 		blindedPath := r.BlindedPayment.BlindedPath.BlindedHops
@@ -1210,7 +1205,6 @@ func lastHopPayloadSize(r *RestrictParams, finalHtlcExpiry int32,
 		finalHop := route.Hop{
 			AmtToForward:     amount,
 			OutgoingTimeLock: uint32(finalHtlcExpiry),
-			LegacyPayload:    false,
 			EncryptedData:    encryptedData,
 		}
 		if len(blindedPath) == 1 {
@@ -1238,7 +1232,6 @@ func lastHopPayloadSize(r *RestrictParams, finalHtlcExpiry int32,
 		AmtToForward:     amount,
 		OutgoingTimeLock: uint32(finalHtlcExpiry),
 		CustomRecords:    r.DestCustomRecords,
-		LegacyPayload:    legacy,
 		MPP:              mpp,
 		AMP:              amp,
 		Metadata:         r.Metadata,

routing/pathfind_test.go

@@ -763,9 +763,6 @@ func TestPathFinding(t *testing.T) {
 	}, {
 		name: "path finding with additional edges",
 		fn:   runPathFindingWithAdditionalEdges,
-	}, {
-		name: "path finding max payload restriction",
-		fn:   runPathFindingMaxPayloadRestriction,
 	}, {
 		name: "path finding with redundant additional edges",
 		fn:   runPathFindingWithRedundantAdditionalEdges,
@@ -1291,122 +1288,6 @@ func runPathFindingWithAdditionalEdges(t *testing.T, useCache bool) {
 	assertExpectedPath(t, graph.aliasMap, path, "songoku", "doge")
 }
 
-// runPathFindingMaxPayloadRestriction tests the maximum size of a sphinx
-// package when creating a route. So we make sure the pathfinder does not return
-// a route which is greater than the maximum sphinx package size of 1300 bytes
-// defined in BOLT04.
-func runPathFindingMaxPayloadRestriction(t *testing.T, useCache bool) {
-	graph, err := parseTestGraph(t, useCache, basicGraphFilePath)
-	require.NoError(t, err, "unable to create graph")
-
-	sourceNode, err := graph.graph.SourceNode()
-	require.NoError(t, err, "unable to fetch source node")
-
-	paymentAmt := lnwire.NewMSatFromSatoshis(100)
-
-	// Create a node doge which is not visible in the graph.
-	dogePubKeyHex := "03dd46ff29a6941b4a2607525b043ec9b020b3f318a1bf281" +
-		"536fd7011ec59c882"
-	dogePubKeyBytes, err := hex.DecodeString(dogePubKeyHex)
-	require.NoError(t, err, "unable to decode public key")
-	dogePubKey, err := btcec.ParsePubKey(dogePubKeyBytes)
-	require.NoError(t, err, "unable to parse public key from bytes")
-
-	doge := &channeldb.LightningNode{}
-	doge.AddPubKey(dogePubKey)
-	doge.Alias = "doge"
-	copy(doge.PubKeyBytes[:], dogePubKeyBytes)
-	graph.aliasMap["doge"] = doge.PubKeyBytes
-
-	const (
-		chanID          uint64 = 1337
-		finalHtlcExpiry int32  = 0
-	)
-
-	// Create the channel edge going from songoku to doge and later add it
-	// with the mocked size function to the graph data.
-	songokuToDogePolicy := &models.CachedEdgePolicy{
-		ToNodePubKey: func() route.Vertex {
-			return doge.PubKeyBytes
-		},
-		ToNodeFeatures:            lnwire.EmptyFeatureVector(),
-		ChannelID:                 chanID,
-		FeeBaseMSat:               1,
-		FeeProportionalMillionths: 1000,
-		TimeLockDelta:             9,
-	}
-
-	// The route has 2 hops. The exit hop (doge) and the hop
-	// (songoku -> doge). The desired path looks like this:
-	// source -> songoku -> doge
-	tests := []struct {
-		name              string
-		mockedPayloadSize uint64
-		err               error
-	}{
-		{
-			// The final hop payload size needs to be considered
-			// as well and because it's treated differently than the
-			// intermediate hops the following tests choose to use
-			// the legacy payload format to have a constant final
-			// hop payload size.
-			name:              "route max payload size (1300)",
-			mockedPayloadSize: 1300 - sphinx.LegacyHopDataSize,
-		},
-		{
-			// We increase the enrypted data size by one byte.
-			name: "route 1 bytes bigger than max " +
-				"payload",
-			mockedPayloadSize: 1300 - sphinx.LegacyHopDataSize + 1,
-			err:               errNoPathFound,
-		},
-	}
-
-	for _, testCase := range tests {
-		testCase := testCase
-
-		t.Run(testCase.name, func(t *testing.T) {
-			t.Parallel()
-
-			restrictions := *noRestrictions
-			// No tlv payload, this makes sure the final hop uses
-			// the legacy payload.
-			restrictions.DestFeatures = lnwire.EmptyFeatureVector()
-
-			// Create the mocked AdditionalEdge and mock the
-			// corresponding calls.
-			mockedEdge := &mockAdditionalEdge{}
-
-			mockedEdge.On("EdgePolicy").Return(songokuToDogePolicy)
-
-			mockedEdge.On("IntermediatePayloadSize",
-				paymentAmt, uint32(finalHtlcExpiry), true,
-				chanID).Once().
-				Return(testCase.mockedPayloadSize)
-
-			additionalEdges := map[route.Vertex][]AdditionalEdge{
-				graph.aliasMap["songoku"]: {mockedEdge},
-			}
-
-			path, err := dbFindPath(
-				graph.graph, additionalEdges,
-				&mockBandwidthHints{}, &restrictions,
-				testPathFindingConfig, sourceNode.PubKeyBytes,
-				doge.PubKeyBytes, paymentAmt, 0,
-				finalHtlcExpiry,
-			)
-			require.ErrorIs(t, err, testCase.err)
-
-			if err == nil {
-				assertExpectedPath(t, graph.aliasMap, path,
-					"songoku", "doge")
-			}
-
-			mockedEdge.AssertExpectations(t)
-		})
-	}
-}
-
 // runPathFindingWithRedundantAdditionalEdges asserts that we are able to find
 // paths to nodes ignoring additional edges that are already known by self node.
 func runPathFindingWithRedundantAdditionalEdges(t *testing.T, useCache bool) {
@@ -1464,7 +1345,6 @@ func runPathFindingWithRedundantAdditionalEdges(t *testing.T, useCache bool) {
 // TestNewRoute tests whether the construction of hop payloads by newRoute
 // is executed correctly.
 func TestNewRoute(t *testing.T) {
-
 	var sourceKey [33]byte
 	sourceVertex := route.Vertex(sourceKey)
 
@@ -1541,8 +1421,6 @@ func TestNewRoute(t *testing.T) {
 		// expectError is true.
 		expectedErrorCode errorCode
 
-		expectedTLVPayload bool
-
 		expectedMPP *record.MPP
 	}{
 		{
@@ -1586,7 +1464,6 @@ func TestNewRoute(t *testing.T) {
 			expectedTimeLocks:     []uint32{1, 1},
 			expectedTotalAmount:   100130,
 			expectedTotalTimeLock: 6,
-			expectedTLVPayload:    true,
 		}, {
 			// For a two hop payment, only the fee for the first hop
 			// needs to be paid. The destination hop does not require
@@ -1603,7 +1480,6 @@ func TestNewRoute(t *testing.T) {
 			expectedTimeLocks:     []uint32{1, 1},
 			expectedTotalAmount:   100130,
 			expectedTotalTimeLock: 6,
-			expectedTLVPayload:    true,
 			expectedMPP: record.NewMPP(
 				100000, testPaymentAddr,
 			),
@@ -1717,14 +1593,6 @@ func TestNewRoute(t *testing.T) {
 			}
 
 			finalHop := route.Hops[len(route.Hops)-1]
-			if !finalHop.LegacyPayload !=
-				testCase.expectedTLVPayload {
-
-				t.Errorf("Expected final hop tlv payload: %t, "+
-					"but got: %t instead",
-					testCase.expectedTLVPayload,
-					!finalHop.LegacyPayload)
-			}
 
 			if !reflect.DeepEqual(
 				finalHop.MPP, testCase.expectedMPP,
@@ -1787,9 +1655,9 @@ func TestNewRoute(t *testing.T) {
 func runNewRoutePathTooLong(t *testing.T, useCache bool) {
 	var testChannels []*testChannel
 
-	// Setup a linear network of 21 hops.
+	// Setup a linear network of 26 hops.
 	fromNode := "start"
-	for i := 0; i < 21; i++ {
+	for i := 0; i < 26; i++ {
 		toNode := fmt.Sprintf("node-%v", i+1)
 		c := symmetricTestChannel(fromNode, toNode, 100000, &testChannelPolicy{
 			Expiry:  144,
@@ -1804,18 +1672,16 @@ func runNewRoutePathTooLong(t *testing.T, useCache bool) {
 
 	ctx := newPathFindingTestContext(t, useCache, testChannels, "start")
 
-	// Assert that we can find 20 hop routes.
+	// Assert that we can find 25 hop routes.
-	node20 := ctx.keyFromAlias("node-20")
+	node25 := ctx.keyFromAlias("node-25")
 	payAmt := lnwire.MilliSatoshi(100001)
-	_, err := ctx.findPath(node20, payAmt)
+	_, err := ctx.findPath(node25, payAmt)
 	require.NoError(t, err, "unexpected pathfinding failure")
 
-	// Assert that finding a 21 hop route fails.
+	// Assert that finding a 26 hop route fails.
-	node21 := ctx.keyFromAlias("node-21")
+	node26 := ctx.keyFromAlias("node-26")
-	_, err = ctx.findPath(node21, payAmt)
+	_, err = ctx.findPath(node26, payAmt)
-	if err != errNoPathFound {
+	require.ErrorIs(t, err, errNoPathFound)
-		t.Fatalf("not route error expected, but got %v", err)
-	}
 
 	// Assert that we can't find a 20 hop route if custom records make it
 	// exceed the maximum payload size.
@@ -1823,7 +1689,7 @@ func runNewRoutePathTooLong(t *testing.T, useCache bool) {
 	ctx.restrictParams.DestCustomRecords = map[uint64][]byte{
 		100000: bytes.Repeat([]byte{1}, 100),
 	}
-	_, err = ctx.findPath(node20, payAmt)
+	_, err = ctx.findPath(node25, payAmt)
 	if err != errNoPathFound {
 		t.Fatalf("not route error expected, but got %v", err)
 	}
@@ -3675,7 +3541,6 @@ func TestLastHopPayloadSize(t *testing.T) {
 		restrictions   *RestrictParams
 		finalHopExpiry int32
 		amount         lnwire.MilliSatoshi
-		legacy         bool
 	}{
 		{
 			name: "Non blinded final hop",
@@ -3687,22 +3552,6 @@ func TestLastHopPayloadSize(t *testing.T) {
 			},
 			amount:         amtToForward,
 			finalHopExpiry: finalHopExpiry,
-			legacy:         false,
-		},
-		{
-			name: "Non blinded final hop legacy",
-			restrictions: &RestrictParams{
-				// The legacy encoding has no ability to include
-				// those extra data we expect that this data is
-				// ignored.
-				PaymentAddr:       paymentAddr,
-				DestCustomRecords: customRecords,
-				Metadata:          metadata,
-				Amp:               ampOptions,
-			},
-			amount:         amtToForward,
-			finalHopExpiry: finalHopExpiry,
-			legacy:         true,
 		},
 		{
 			name: "Blinded final hop introduction point",
@@ -3754,7 +3603,6 @@ func TestLastHopPayloadSize(t *testing.T) {
 				finalHop = route.Hop{
 					AmtToForward:     tc.amount,
 					OutgoingTimeLock: uint32(tc.finalHopExpiry),
-					LegacyPayload:    false,
 					EncryptedData:    blindedPath[len(blindedPath)-1].CipherText,
 				}
 				if len(blindedPath) == 1 {
@@ -3763,7 +3611,6 @@ func TestLastHopPayloadSize(t *testing.T) {
 			} else {
 				//nolint:lll
 				finalHop = route.Hop{
-					LegacyPayload:    tc.legacy,
 					AmtToForward:     tc.amount,
 					OutgoingTimeLock: uint32(tc.finalHopExpiry),
 					Metadata:         tc.restrictions.Metadata,
@@ -3778,7 +3625,7 @@ func TestLastHopPayloadSize(t *testing.T) {
 
 			expectedPayloadSize := lastHopPayloadSize(
 				tc.restrictions, tc.finalHopExpiry,
-				tc.amount, tc.legacy,
+				tc.amount,
 			)
 
 			require.Equal(

routing/route/route.go

@@ -133,6 +133,11 @@ type Hop struct {
 	// LegacyPayload if true, then this signals that this node doesn't
 	// understand the new TLV payload, so we must instead use the legacy
 	// payload.
+	//
+	// NOTE: we should no longer ever create a Hop with Legacy set to true.
+	// The only reason we are keeping this member is that it could be the
+	// case that we have serialised hops persisted to disk where
+	// LegacyPayload is true.
 	LegacyPayload bool
 
 	// Metadata is additional data that is sent along with the payment to
@@ -190,7 +195,7 @@ func (h *Hop) PackHopPayload(w io.Writer, nextChanID uint64,
 
 	// If this is a legacy payload, then we'll exit here as this method
 	// shouldn't be called.
-	if h.LegacyPayload == true {
+	if h.LegacyPayload {
 		return fmt.Errorf("cannot pack hop payloads for legacy " +
 			"payloads")
 	}
@@ -370,8 +375,8 @@ func validateNextChanID(nextChanIDIsSet, isBlinded, finalHop bool) error {
 	}
 }
 
-// Size returns the total size this hop's payload would take up in the onion
+// PayloadSize returns the total size this hop's payload would take up in the
-// packet.
+// onion packet.
 func (h *Hop) PayloadSize(nextChanID uint64) uint64 {
 	if h.LegacyPayload {
 		return sphinx.LegacyHopDataSize