routing/pathfind: fix TLV fallback for unadvertised hops

In this commit, we fix a bug that prevents us from sending custom
records to nodes that aren't in the graph. Previously we would simply
fail if we were unable to retrieve the node's features.

To remedy, we add the option of supplying the destination's feature bits
into path finding. If present, we will use them directly without
consulting the graph, resolving the original issue. Instead, we will
only consult the graph as a fallback, which will still fail if the node
doesn't exist since the TLV features won't be populated in the empty
feature vector.

Furthermore, this also permits us to provide "virtual features" into the
pathfinding logic, where we make assumptions about what the receiver
supports even if the feature vector isn't actually taken from an
invoice. This can useful in cases like keysend, where we don't have an
invoice, but we can still attempt the payment if we assume the receiver
supports TLV.

routing/pathfind.go

@@ -292,6 +292,11 @@ type RestrictParams struct {
 	// DestCustomRecords contains the custom records to drop off at the
 	// final hop, if any.
 	DestCustomRecords record.CustomSet
+
+	// DestFeatures is a feature vector describing what the final hop
+	// supports. If none are provided, pathfinding will try to inspect any
+	// features on the node announcement instead.
+	DestFeatures *lnwire.FeatureVector
 }
 
 // PathFindingConfig defines global parameters that control the trade-off in
@@ -395,29 +400,41 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
 		defer tx.Rollback()
 	}
 
-	if len(r.DestCustomRecords) > 0 {
-		// Check if the target has TLV enabled
-
+	// If no destination features are provided, we will load what features
+	// we have for the target node from our graph.
+	features := r.DestFeatures
+	if features == nil {
 		targetKey, err := btcec.ParsePubKey(target[:], btcec.S256())
 		if err != nil {
 			return nil, err
 		}
 
 		targetNode, err := g.graph.FetchLightningNode(targetKey)
-		if err != nil {
-			return nil, err
-		}
+		switch {
 
-		if targetNode.Features != nil {
-			supportsTLV := targetNode.Features.HasFeature(
-				lnwire.TLVOnionPayloadOptional,
-			)
-			if !supportsTLV {
-				return nil, errNoTlvPayload
-			}
+		// If the node exists and has features, use them directly.
+		case err == nil:
+			features = targetNode.Features
+
+		// If an error other than the node not existing is hit, abort.
+		case err != channeldb.ErrGraphNodeNotFound:
+			return nil, err
+
+		// Otherwise, we couldn't find a node announcement, populate a
+		// blank feature vector.
+		default:
+			features = lnwire.EmptyFeatureVector()
 		}
 	}
 
+	// If the caller needs to send custom records, check that our
+	// destination feature vector supports TLV.
+	if len(r.DestCustomRecords) > 0 &&
+		!features.HasFeature(lnwire.TLVOnionPayloadOptional) {
+
+		return nil, errNoTlvPayload
+	}
+
 	// If we are routing from ourselves, check that we have enough local
 	// balance available.
 	if source == self {

routing/pathfind_test.go

@@ -23,6 +23,7 @@ import (
 	"github.com/btcsuite/btcutil"
 	"github.com/lightningnetwork/lnd/channeldb"
 	"github.com/lightningnetwork/lnd/lnwire"
+	"github.com/lightningnetwork/lnd/record"
 	"github.com/lightningnetwork/lnd/routing/route"
 	"github.com/lightningnetwork/lnd/zpay32"
 )
@@ -58,6 +59,12 @@ var (
 	}
 
 	testPathFindingConfig = &PathFindingConfig{}
+
+	tlvFeatures = lnwire.NewFeatureVector(
+		lnwire.NewRawFeatureVector(
+			lnwire.TLVOnionPayloadOptional,
+		), lnwire.Features,
+	)
 )
 
 var (
@@ -917,6 +924,10 @@ func testBasicGraphPathFindingCase(t *testing.T, graphInstance *testGraphInstanc
 	}
 }
 
+// TestPathFindingWithAdditionalEdges asserts that we are able to find paths to
+// nodes that do not exist in the graph by way of hop hints. We also test that
+// the path can support custom TLV records for the receiver under the
+// appropriate circumstances.
 func TestPathFindingWithAdditionalEdges(t *testing.T) {
 	t.Parallel()
 
@@ -968,15 +979,21 @@ func TestPathFindingWithAdditionalEdges(t *testing.T) {
 		graph.aliasMap["songoku"]: {songokuToDoge},
 	}
 
+	find := func(r *RestrictParams) (
+		[]*channeldb.ChannelEdgePolicy, error) {
+
+		return findPath(
+			&graphParams{
+				graph:           graph.graph,
+				additionalEdges: additionalEdges,
+			},
+			r, testPathFindingConfig,
+			sourceNode.PubKeyBytes, doge.PubKeyBytes, paymentAmt,
+		)
+	}
+
 	// We should now be able to find a path from roasbeef to doge.
-	path, err := findPath(
-		&graphParams{
-			graph:           graph.graph,
-			additionalEdges: additionalEdges,
-		},
-		noRestrictions, testPathFindingConfig,
-		sourceNode.PubKeyBytes, doge.PubKeyBytes, paymentAmt,
-	)
+	path, err := find(noRestrictions)
 	if err != nil {
 		t.Fatalf("unable to find private path to doge: %v", err)
 	}
@@ -984,6 +1001,35 @@ func TestPathFindingWithAdditionalEdges(t *testing.T) {
 	// The path should represent the following hops:
 	//	roasbeef -> songoku -> doge
 	assertExpectedPath(t, graph.aliasMap, path, "songoku", "doge")
+
+	// Now, set custom records for the final hop. This should fail since no
+	// dest features are set, and we won't have a node ann to fall back on.
+	restrictions := *noRestrictions
+	restrictions.DestCustomRecords = record.CustomSet{70000: []byte{}}
+
+	_, err = find(&restrictions)
+	if err != errNoTlvPayload {
+		t.Fatalf("path shouldn't have been found: %v", err)
+	}
+
+	// Set empty dest features so we don't try the fallback. We should still
+	// fail since the tlv feature isn't set.
+	restrictions.DestFeatures = lnwire.EmptyFeatureVector()
+
+	_, err = find(&restrictions)
+	if err != errNoTlvPayload {
+		t.Fatalf("path shouldn't have been found: %v", err)
+	}
+
+	// Finally, set the tlv feature in the payload and assert we found the
+	// same path as before.
+	restrictions.DestFeatures = tlvFeatures
+
+	path, err = find(&restrictions)
+	if err != nil {
+		t.Fatalf("path should have been found: %v", err)
+	}
+	assertExpectedPath(t, graph.aliasMap, path, "songoku", "doge")
 }
 
 // TestNewRoute tests whether the construction of hop payloads by newRoute
@@ -1296,6 +1342,110 @@ func TestPathNotAvailable(t *testing.T) {
 	}
 }
 
+// TestDestTLVGraphFallback asserts that we properly detect when we can send TLV
+// records to a receiver, and also that we fallback to the receiver's node
+// announcement if we don't have an invoice features.
+func TestDestTLVGraphFallback(t *testing.T) {
+	t.Parallel()
+
+	testChannels := []*testChannel{
+		asymmetricTestChannel("roasbeef", "luoji", 100000,
+			&testChannelPolicy{
+				Expiry:  144,
+				FeeRate: 400,
+				MinHTLC: 1,
+				MaxHTLC: 100000000,
+			}, &testChannelPolicy{
+				Expiry:  144,
+				FeeRate: 400,
+				MinHTLC: 1,
+				MaxHTLC: 100000000,
+			}, 0),
+		asymmetricTestChannel("roasbeef", "satoshi", 100000,
+			&testChannelPolicy{
+				Expiry:  144,
+				FeeRate: 400,
+				MinHTLC: 1,
+				MaxHTLC: 100000000,
+			}, &testChannelPolicy{
+				Expiry:   144,
+				FeeRate:  400,
+				MinHTLC:  1,
+				MaxHTLC:  100000000,
+				Features: tlvFeatures,
+			}, 0),
+	}
+
+	ctx := newPathFindingTestContext(t, testChannels, "roasbeef")
+	defer ctx.cleanup()
+
+	sourceNode, err := ctx.graphParams.graph.SourceNode()
+	if err != nil {
+		t.Fatalf("unable to fetch source node: %v", err)
+
+	}
+
+	find := func(r *RestrictParams,
+		target route.Vertex) ([]*channeldb.ChannelEdgePolicy, error) {
+
+		return findPath(
+			&graphParams{
+				graph: ctx.graphParams.graph,
+			},
+			r, testPathFindingConfig,
+			sourceNode.PubKeyBytes, target, 100,
+		)
+	}
+
+	// Luoji's node ann has an empty feature vector.
+	luoji := ctx.testGraphInstance.aliasMap["luoji"]
+
+	// Satoshi's node ann supports TLV.
+	satoshi := ctx.testGraphInstance.aliasMap["satoshi"]
+
+	restrictions := *noRestrictions
+
+	// Add custom records w/o any dest features.
+	restrictions.DestCustomRecords = record.CustomSet{70000: []byte{}}
+
+	// Path to luoji should fail because his node ann features are empty.
+	_, err = find(&restrictions, luoji)
+	if err != errNoTlvPayload {
+		t.Fatalf("path shouldn't have been found: %v", err)
+	}
+
+	// However, path to satoshi should succeed via the fallback because his
+	// node ann features have the TLV bit.
+	path, err := find(&restrictions, satoshi)
+	if err != nil {
+		t.Fatalf("path should have been found: %v", err)
+	}
+	assertExpectedPath(t, ctx.testGraphInstance.aliasMap, path, "satoshi")
+
+	// Add empty destination features. This should cause both paths to fail,
+	// since this override anything in the graph.
+	restrictions.DestFeatures = lnwire.EmptyFeatureVector()
+
+	_, err = find(&restrictions, luoji)
+	if err != errNoTlvPayload {
+		t.Fatalf("path shouldn't have been found: %v", err)
+	}
+	_, err = find(&restrictions, satoshi)
+	if err != errNoTlvPayload {
+		t.Fatalf("path shouldn't have been found: %v", err)
+	}
+
+	// Finally, set the TLV dest feature. We should succeed in finding a
+	// path to luoji.
+	restrictions.DestFeatures = tlvFeatures
+
+	path, err = find(&restrictions, luoji)
+	if err != nil {
+		t.Fatalf("path should have been found: %v", err)
+	}
+	assertExpectedPath(t, ctx.testGraphInstance.aliasMap, path, "luoji")
+}
+
 func TestPathInsufficientCapacity(t *testing.T) {
 	t.Parallel()