Merge pull request #10396 from ziggie1984/enhance-lsp-heuritic

Enhance Lsp Heuristic when probing a payment

Author: ellemouton

docs/estimate_route_fee.md

@@ -150,52 +150,58 @@ probing.
 
 The heuristic examines the structure of route hints provided in the invoice to
 identify characteristic LSP patterns. The detection operates on the principle
-that LSPs typically maintain private channels to their users and appear as the
-penultimate hop in payment routing.
+that LSPs typically maintain private channels to their users and appear as
+public nodes in the network, while the final destination is private.
 
 ```mermaid
 flowchart TD
     Start([Route Hints Received]) --> Empty{Empty Hints?}
     Empty -->|Yes| NotLSP([Not LSP])
-    Empty -->|No| GetFirst[Get First Hint's Last Hop]
-    
-    GetFirst --> CheckPub1{Is Channel<br/>Public?}
-    CheckPub1 -->|Yes| NotLSP
-    CheckPub1 -->|No| SaveNode[Save Node ID]
-    
-    SaveNode --> MoreHints{More Hints?}
-    MoreHints -->|No| IsLSP([Detected as LSP])
+    Empty -->|No| CheckTarget{Invoice Target<br/>in Graph?}
+
+    CheckTarget -->|Yes| NotLSP
+    CheckTarget -->|No| GetFirstDest[Get First Hint's<br/>Destination Hop]
+
+    GetFirstDest --> CheckPub1{Destination Node<br/>in Graph?}
+    CheckPub1 -->|Yes| IsLSP([Detected as LSP])
+    CheckPub1 -->|No| MoreHints{More Hints?}
+
+    MoreHints -->|No| NotLSP
     MoreHints -->|Yes| NextHint[Check Next Hint]
-    
-    NextHint --> GetLast[Get Last Hop]
-    GetLast --> CheckPub2{Is Channel<br/>Public?}
-    CheckPub2 -->|Yes| NotLSP
-    CheckPub2 -->|No| SameNode{Same Node ID<br/>as First?}
-    
-    SameNode -->|No| NotLSP
-    SameNode -->|Yes| MoreHints
-```
 
-The detection criteria are:
+    NextHint --> GetNextDest[Get Destination Hop]
+    GetNextDest --> CheckPub2{Destination Node<br/>in Graph?}
+    CheckPub2 -->|Yes| IsLSP
+    CheckPub2 -->|No| MoreHints
+```
 
-- **All route hints must terminate at the same node ID** - This indicates a
-  single destination behind potentially multiple LSP entry points
+The detection follows three simple rules applied sequentially:
 
-- **Final hop channels must be private** - The channels in the last hop of
-  each route hint must not exist in the public channel graph
+**Rule 1: Public Invoice Target → NOT an LSP**
+- If the invoice target (destination) is a public node that exists in the
+  channel graph, the payment can be routed directly to it
+- This means it's not an LSP setup, regardless of what route hints are provided
+- Example: A well-connected merchant node with route hints for liquidity
+  signaling
 
-- **No public channels in final hops** - If any route hint contains a public
-  channel in its final hop, LSP detection is disabled entirely
+**Rule 2: Public Destination Hop → IS an LSP**
+- If at least one route hint has a destination hop (last hop in the route hint)
+  that is a public node in the graph, LSP detection is triggered
+- This indicates the destination hop is an LSP serving a private client
+- The private client is reached through the LSP's private channel
 
-- **Multiple route hints strengthen detection** - While not required,
-  multiple hints converging on the same destination strongly suggest an LSP
-  configuration
+**Rule 3: All Private Destination Hops → NOT an LSP**
+- If all destination hops in all route hints are private nodes (not in the
+  public graph), this is not treated as an LSP setup
+- The payment will be routed directly to the invoice destination using the
+  route hints as additional path information
+- This is the standard case for private channel payments
 
 This pattern effectively distinguishes LSP configurations from other routing
 scenarios. For instance, some Lightning implementations like CLN include route
 hints even for public nodes to signal liquidity availability or preferred
-routing paths. The heuristic correctly identifies these as non-LSP scenarios by
-detecting the presence of public channels.
+routing paths. The heuristic correctly identifies these as non-LSP scenarios
+by Rule 1 (detecting that the invoice target itself is public).
 
 ### How Probing Differs When an LSP is Detected
 
@@ -432,10 +438,6 @@ appropriately.
 The `EstimateRouteFee` implementation continues to evolve based on real-world
 usage patterns. Ongoing discussions in the LND community focus on:
 
-**Improved LSP Detection**: Developing more sophisticated heuristics that
-accurately identify LSP configurations while avoiding false positives for
-regular private channels.
-
 **Multi-Path Payment Support**: Extending fee estimation to support MPP
 scenarios where payments split across multiple routes.
 

docs/release-notes/release-notes-0.20.1.md

@@ -55,6 +55,14 @@
 
 ## RPC Updates
 
+ * The `EstimateRouteFee` RPC now implements an [LSP detection 
+   heuristic](https://github.com/lightningnetwork/lnd/pull/10396) that probes up
+   to 3 unique Lightning Service Providers when route hints indicate an LSP
+   setup. The implementation returns worst-case (most expensive) fee estimates
+   for conservative budgeting and includes griefing protection by limiting the
+   number of probed LSPs. It enhances the previous LSP design by being more
+   generic and more flexible.
+
 ## lncli Updates
 
 ## Breaking Changes

graph/db/kv_store.go

@@ -3431,11 +3431,10 @@ func (c *KVStore) fetchLightningNode(tx kvdb.RTx,
 	return node, nil
 }
 
-// HasLightningNode determines if the graph has a vertex identified by the
-// target node identity public key. If the node exists in the database, a
-// timestamp of when the data for the node was lasted updated is returned along
-// with a true boolean. Otherwise, an empty time.Time is returned with a false
-// boolean.
+// HasNode determines if the graph has a vertex identified by the target node
+// identity public key. If the node exists in the database, a timestamp of when
+// the data for the node was lasted updated is returned along with a true
+// boolean. Otherwise, an empty time.Time is returned with a false boolean.
 func (c *KVStore) HasNode(_ context.Context,
 	nodePub [33]byte) (time.Time, bool, error) {
 

itest/lnd_estimate_route_fee_test.go

@@ -59,18 +59,38 @@ type estimateRouteFeeTestCase struct {
 }
 
 // testEstimateRouteFee tests the estimation of routing fees using either graph
-// data or sending out a probe payment.
+// data or sending out a probe payment. This test validates graph-based fee
+// estimation, probe-based fee estimation with single LSP, probe-based fee
+// estimation with multiple route hints to same LSP (worst-case fee selection),
+// probe-based fee estimation with multiple different public LSPs (worst-case
+// fee selection across LSPs, up to MaxLspsToProbe), and non-LSP probing (all
+// private destination hops).
+//
+// Note: We test with exactly MaxLspsToProbe (3) LSPs. Testing with more LSPs
+// is not feasible because the LSP selection uses map iteration, which has
+// non-deterministic order in Go, making it impossible to predict which LSPs
+// will be probed.
 func testEstimateRouteFee(ht *lntest.HarnessTest) {
+	// Ensure MaxLspsToProbe is set to 3 as expected by this test. The test
+	// uses exactly 3 LSPs in the multi-LSP test case. If MaxLspsToProbe
+	// changes, this assertion will fail as a reminder to update the test.
+	require.Equal(ht, 3, routerrpc.MaxLspsToProbe,
+		"MaxLspsToProbe should be 3")
+
 	mts := newMppTestScenario(ht)
 
-	// We extend the regular mpp test scenario with a new node Paula. Paula
-	// is connected to Bob and Eve through private channels.
+	// We extend the regular mpp test scenario with two new nodes:
+	// - Paula: connected to Bob and Eve through private channels
+	// - Frank: connected to Dave through a private channel
+	//
 	//                  /-------------\
 	//              _ Eve _  (private) \
 	//             /       \            \
 	// Alice -- Carol ---- Bob --------- Paula
 	//      \              /   (private)
 	//       \__ Dave ____/
+	//             \
+	//              \__ Frank (private)
 	//
 	req := &mppOpenChannelRequest{
 		amtAliceCarol: 200_000,
@@ -88,6 +108,7 @@ func testEstimateRouteFee(ht *lntest.HarnessTest) {
 	probeInitiator = mts.alice
 
 	paula := ht.NewNode("Paula", nil)
+	frank := ht.NewNode("Frank", nil)
 
 	// The channel from Bob to Paula actually doesn't have enough liquidity
 	// to carry out the probe. We assume in normal operation that hop hints
@@ -106,6 +127,13 @@ func testEstimateRouteFee(ht *lntest.HarnessTest) {
 		Amt:     1_000_000,
 	})
 
+	// Frank is a private node connected to Dave (public LSP).
+	ht.EnsureConnected(mts.dave, frank)
+	ht.OpenChannel(mts.dave, frank, lntest.OpenChannelParams{
+		Private: true,
+		Amt:     1_000_000,
+	})
+
 	bobsPrivChannels := mts.bob.RPC.ListChannels(&lnrpc.ListChannelsRequest{
 		PrivateOnly: true,
 	})
@@ -118,6 +146,14 @@ func testEstimateRouteFee(ht *lntest.HarnessTest) {
 	require.Len(ht, evesPrivChannels.Channels, 1)
 	evePaulaChanID := evesPrivChannels.Channels[0].ChanId
 
+	davesPrivChannels := mts.dave.RPC.ListChannels(
+		&lnrpc.ListChannelsRequest{
+			PrivateOnly: true,
+		},
+	)
+	require.Len(ht, davesPrivChannels.Channels, 1)
+	daveFrankChanID := davesPrivChannels.Channels[0].ChanId
+
 	// Let's disable the paths from Alice to Bob through Dave and Eve with
 	// high fees. This ensures that the path estimates are based on Carol's
 	// channel to Bob for the first set of tests.
@@ -196,6 +232,33 @@ func testEstimateRouteFee(ht *lntest.HarnessTest) {
 				},
 			},
 		}
+
+		daveHopHint = &lnrpc.HopHint{
+			NodeId:                    mts.dave.PubKeyStr,
+			FeeBaseMsat:               3_000,
+			FeeProportionalMillionths: 3_000,
+			CltvExpiryDelta:           120,
+			ChanId:                    daveFrankChanID,
+		}
+
+		// Multiple different public LSPs (Bob, Eve, Dave).
+		multipleLspsRouteHints = []*lnrpc.RouteHint{
+			{
+				HopHints: []*lnrpc.HopHint{
+					bobHopHint,
+				},
+			},
+			{
+				HopHints: []*lnrpc.HopHint{
+					eveHopHint,
+				},
+			},
+			{
+				HopHints: []*lnrpc.HopHint{
+					daveHopHint,
+				},
+			},
+		}
 	)
 
 	defaultTimelock := int64(chainreg.DefaultBitcoinTimeLockDelta)
@@ -231,6 +294,14 @@ func testEstimateRouteFee(ht *lntest.HarnessTest) {
 	feeACEP := feeEP + feeCE
 	deltaACEP := deltaCE + deltaEP
 
+	// For multiple LSPs test, the route with the highest fee should be
+	// selected (Eve). Note that we return both fee and CLTV delta from
+	// the same route (the highest-fee route), not the max fee and max
+	// delta independently. This ensures the returned values represent an
+	// actual viable route.
+	highestFeeRouteFee := feeACEP
+	highestFeeRouteDelta := deltaACEP
+
 	initialBlockHeight := int64(mts.alice.RPC.GetInfo().BlockHeight)
 
 	// Locktime is always composed of the initial block height and the
@@ -271,6 +342,19 @@ func testEstimateRouteFee(ht *lntest.HarnessTest) {
 			expectedCltvDelta:       locktime + deltaCB,
 			expectedFailureReason:   failureReasonNone,
 		},
+		// Rule 1: Invoice target is public (Bob), even with public
+		// destination hop hints. Should route directly to Bob, NOT
+		// treat as LSP.
+		{
+			name: "probe based estimate, public " +
+				"target with public hop hints",
+			probing:                 true,
+			destination:             mts.bob,
+			routeHints:              singleRouteHint,
+			expectedRoutingFeesMsat: feeStandardSingleHop,
+			expectedCltvDelta:       locktime + deltaCB,
+			expectedFailureReason:   failureReasonNone,
+		},
 		// We expect the previous probing results adjusted by Paula's
 		// hop data.
 		{
@@ -340,6 +424,23 @@ func testEstimateRouteFee(ht *lntest.HarnessTest) {
 			expectedCltvDelta:       0,
 			expectedFailureReason:   failureReasonNoRoute,
 		},
+		// Test multiple different public LSPs. The worst-case (most
+		// expensive) route should be returned. Eve has the highest
+		// fees among the 3 LSPs tested. Note: We don't test with more
+		// than MaxLspsToProbe LSPs because map iteration order in Go
+		// is non-deterministic, making it impossible to predict which
+		// LSPs will be selected for probing.
+		{
+			name: "probe based estimate, " +
+				"multiple different public LSPs",
+			probing:                 true,
+			destination:             frank,
+			routeHints:              multipleLspsRouteHints,
+			expectedRoutingFeesMsat: highestFeeRouteFee,
+			expectedCltvDelta: locktime +
+				highestFeeRouteDelta,
+			expectedFailureReason: failureReasonNone,
+		},
 	}
 
 	for _, testCase := range testCases {

lnrpc/routerrpc/router_backend.go

@@ -64,6 +64,11 @@ type RouterBackend struct {
 	FetchChannelEndpoints func(chanID uint64) (route.Vertex,
 		route.Vertex, error)
 
+	// HasNode returns true if the node exists in the graph (i.e., has
+	// public channels), false otherwise. This means the node is a public
+	// node and should be reachable.
+	HasNode func(nodePub route.Vertex) (bool, error)
+
 	// FindRoute is a closure that abstracts away how we locate/query for
 	// routes.
 	FindRoute func(*routing.RouteRequest) (*route.Route, float64, error)