Merge pull request #9433 from hieblmi/estimate-route-fee-fix

routerrpc: fix estimateroutefee for public route hints

docs/release-notes/release-notes-0.19.0.md

@@ -62,7 +62,11 @@
   https://github.com/lightningnetwork/lnd/pull/9253)
 
 * [Fixed a bug](https://github.com/lightningnetwork/lnd/pull/9322) that caused
-    estimateroutefee to ignore the default payment timeout.
+  estimateroutefee to ignore the default payment timeout.
+
+* [Fixed a bug](https://github.com/lightningnetwork/lnd/pull/9433) that caused 
+  `estimateroutefee` to assume probing an LSP when given an invoice with a route 
+  hint containing a public channel to the destination.
 
 * [Fix a bug](https://github.com/lightningnetwork/lnd/pull/9474) where LND would
   fail to persist (and hence, propagate) node announcements containing address 

lnrpc/routerrpc/router_server.go

@@ -171,6 +171,11 @@ var (
 	DefaultRouterMacFilename = "router.macaroon"
 )
 
+// FetchChannelEndpoints returns the pubkeys of both endpoints of the
+// given channel id if it exists in the graph.
+type FetchChannelEndpoints func(chanID uint64) (route.Vertex, route.Vertex,
+	error)
+
 // ServerShell is a shell struct holding a reference to the actual sub-server.
 // It is used to register the gRPC sub-server with the root server before we
 // have the necessary dependencies to populate the actual sub-server.
@@ -552,7 +557,7 @@ func (s *Server) probePaymentRequest(ctx context.Context, paymentRequest string,
 	// If the hints don't indicate an LSP then chances are that our probe
 	// payment won't be blocked along the route to the destination. We send
 	// a probe payment with unmodified route hints.
-	if !isLSP(hints) {
+	if !isLSP(hints, s.cfg.RouterBackend.FetchChannelEndpoints) {
 		probeRequest.RouteHints = invoicesrpc.CreateRPCRouteHints(hints)
 		return s.sendProbePayment(ctx, probeRequest)
 	}
@@ -625,14 +630,27 @@ func (s *Server) probePaymentRequest(ctx context.Context, paymentRequest string,
 }
 
 // isLSP checks if the route hints indicate an LSP. An LSP is indicated with
-// true if the last node in each route hint has the same node id, false
+// true if the destination hop hint in each route hint has the same node id,
-// otherwise.
+// false otherwise. If the destination hop hint of any route hint contains a
-func isLSP(routeHints [][]zpay32.HopHint) bool {
+// public channel, the function returns false because we can directly send a
+// probe to the final destination.
+func isLSP(routeHints [][]zpay32.HopHint,
+	fetchChannelEndpoints FetchChannelEndpoints) bool {
+
 	if len(routeHints) == 0 || len(routeHints[0]) == 0 {
 		return false
 	}
 
-	refNodeID := routeHints[0][len(routeHints[0])-1].NodeID
+	destHopHint := routeHints[0][len(routeHints[0])-1]
+
+	// If the destination hop hint of the first route hint contains a public
+	// channel we can send a probe to it directly, hence we don't signal an
+	// LSP.
+	_, _, err := fetchChannelEndpoints(destHopHint.ChannelID)
+	if err == nil {
+		return false
+	}
+
 	for i := 1; i < len(routeHints); i++ {
 		// Skip empty route hints.
 		if len(routeHints[i]) == 0 {
@@ -640,15 +658,27 @@ func isLSP(routeHints [][]zpay32.HopHint) bool {
 		}
 
 		lastHop := routeHints[i][len(routeHints[i])-1]
+
+		// If the last hop hint of any route hint contains a public
+		// channel we can send a probe to it directly, hence we don't
+		// signal an LSP.
+		_, _, err = fetchChannelEndpoints(lastHop.ChannelID)
+		if err == nil {
+			return false
+		}
+
 		idMatchesRefNode := bytes.Equal(
 			lastHop.NodeID.SerializeCompressed(),
-			refNodeID.SerializeCompressed(),
+			destHopHint.NodeID.SerializeCompressed(),
 		)
 		if !idMatchesRefNode {
 			return false
 		}
 	}
 
+	// We ensured that the destination hop hint doesn't contain a public
+	// channel, and that all destination hop hints of all route hints match,
+	// so we signal an LSP.
 	return true
 }
 

lnrpc/routerrpc/router_server_test.go

@@ -7,10 +7,12 @@ import (
 
 	"github.com/btcsuite/btcd/btcec/v2"
 	"github.com/lightningnetwork/lnd/channeldb"
+	graphdb "github.com/lightningnetwork/lnd/graph/db"
 	"github.com/lightningnetwork/lnd/lnrpc"
 	"github.com/lightningnetwork/lnd/lnwire"
 	"github.com/lightningnetwork/lnd/queue"
 	"github.com/lightningnetwork/lnd/routing"
+	"github.com/lightningnetwork/lnd/routing/route"
 	"github.com/lightningnetwork/lnd/zpay32"
 	"github.com/stretchr/testify/require"
 	"google.golang.org/grpc"
@@ -269,6 +271,14 @@ func TestIsLsp(t *testing.T) {
 			FeeProportionalMillionths: 2_000,
 			ChannelID:                 815,
 		}
+
+		publicChannelID       = uint64(42)
+		daveHopHintPublicChan = zpay32.HopHint{
+			NodeID:                    davePubKey,
+			FeeBaseMSat:               2_000,
+			FeeProportionalMillionths: 2_000,
+			ChannelID:                 publicChannelID,
+		}
 	)
 
 	bobExpensiveCopy := bobHopHint.Copy()
@@ -383,11 +393,56 @@ func TestIsLsp(t *testing.T) {
 			expectedHints:  [][]zpay32.HopHint{},
 			expectedLspHop: nil,
 		},
+		{
+			name: "multiple routes, same public hops",
+			routeHints: [][]zpay32.HopHint{
+				{
+					aliceHopHint, daveHopHintPublicChan,
+				},
+				{
+					carolHopHint, daveHopHintPublicChan,
+				},
+			},
+			probeAmtMsat:   probeAmtMsat,
+			isLsp:          false,
+			expectedHints:  [][]zpay32.HopHint{},
+			expectedLspHop: nil,
+		},
+		{
+			name: "multiple routes, same public hops",
+			routeHints: [][]zpay32.HopHint{
+				{
+					aliceHopHint, daveHopHint,
+				},
+				{
+					carolHopHint, daveHopHintPublicChan,
+				},
+				{
+					aliceHopHint, daveHopHintPublicChan,
+				},
+			},
+			probeAmtMsat:   probeAmtMsat,
+			isLsp:          false,
+			expectedHints:  [][]zpay32.HopHint{},
+			expectedLspHop: nil,
+		},
+	}
+
+	// Returns ErrEdgeNotFound for private channels.
+	fetchChannelEndpoints := func(chanID uint64) (route.Vertex,
+		route.Vertex, error) {
+
+		if chanID == publicChannelID {
+			return route.Vertex{}, route.Vertex{}, nil
+		}
+
+		return route.Vertex{}, route.Vertex{}, graphdb.ErrEdgeNotFound
 	}
 
 	for _, tc := range lspTestCases {
 		t.Run(tc.name, func(t *testing.T) {
-			require.Equal(t, tc.isLsp, isLSP(tc.routeHints))
+			isLsp := isLSP(tc.routeHints, fetchChannelEndpoints)
+			require.Equal(t, tc.isLsp, isLsp)
 			if !tc.isLsp {
 				return
 			}