diff --git a/routing/additional_edge.go b/routing/additional_edge.go index 22cf3032b..eee17cce1 100644 --- a/routing/additional_edge.go +++ b/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) } diff --git a/routing/additional_edge_test.go b/routing/additional_edge_test.go index b3ea3b501..b5628c6b7 100644 --- a/routing/additional_edge_test.go +++ b/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, ) diff --git a/routing/blinding_test.go b/routing/blinding_test.go index 69b25c696..f6327ecd5 100644 --- a/routing/blinding_test.go +++ b/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) diff --git a/routing/mocks.go b/routing/mocks.go deleted file mode 100644 index 9c019abc9..000000000 --- a/routing/mocks.go +++ /dev/null @@ -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) -} diff --git a/routing/pathfind.go b/routing/pathfind.go index add833ecc..893cef12a 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -659,8 +659,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, - !features.HasFeature(lnwire.TLVOnionPayloadOptional)) + lastHopPayloadSize := lastHopPayloadSize(r, finalHtlcExpiry, amt) // We can't always assume that the end destination is publicly // advertised to the network so we'll manually include the target node. @@ -882,14 +881,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, ) } @@ -1176,7 +1171,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 @@ -1186,7 +1181,6 @@ func lastHopPayloadSize(r *RestrictParams, finalHtlcExpiry int32, finalHop := route.Hop{ AmtToForward: amount, OutgoingTimeLock: uint32(finalHtlcExpiry), - LegacyPayload: false, EncryptedData: encryptedData, } if len(blindedPath) == 1 { @@ -1214,7 +1208,6 @@ func lastHopPayloadSize(r *RestrictParams, finalHtlcExpiry int32, AmtToForward: amount, OutgoingTimeLock: uint32(finalHtlcExpiry), CustomRecords: r.DestCustomRecords, - LegacyPayload: legacy, MPP: mpp, AMP: amp, Metadata: r.Metadata, diff --git a/routing/pathfind_test.go b/routing/pathfind_test.go index 57f723ce6..af2e296bd 100644 --- a/routing/pathfind_test.go +++ b/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. - node20 := ctx.keyFromAlias("node-20") + // Assert that we can find 25 hop routes. + 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. - node21 := ctx.keyFromAlias("node-21") - _, err = ctx.findPath(node21, payAmt) - if err != errNoPathFound { - t.Fatalf("not route error expected, but got %v", err) - } + // Assert that finding a 26 hop route fails. + node26 := ctx.keyFromAlias("node-26") + _, err = ctx.findPath(node26, payAmt) + require.ErrorIs(t, err, errNoPathFound) // 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( diff --git a/routing/route/route.go b/routing/route/route.go index 934a36ec1..0516cc7a2 100644 --- a/routing/route/route.go +++ b/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 -// packet. +// PayloadSize returns the total size this hop's payload would take up in the +// onion packet. func (h *Hop) PayloadSize(nextChanID uint64) uint64 { if h.LegacyPayload { return sphinx.LegacyHopDataSize