From acb7b83eadf9e4ac615d73dcaea8762b51ccefcc Mon Sep 17 00:00:00 2001
From: Conner Fromknecht <conner@lightning.engineering>
Date: Wed, 18 Dec 2019 23:54:11 -0800
Subject: [PATCH] routing/pathfind: validate final hop feature dependencies

---
 feature/deps.go          |  7 +++-
 routing/pathfind.go      | 12 ++++++
 routing/pathfind_test.go | 86 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 104 insertions(+), 1 deletion(-)

diff --git a/feature/deps.go b/feature/deps.go
index 173318d5..6382ff33 100644
--- a/feature/deps.go
+++ b/feature/deps.go
@@ -30,6 +30,11 @@ type ErrMissingFeatureDep struct {
 	dep lnwire.FeatureBit
 }
 
+// NewErrMissingFeatureDep creates a new ErrMissingFeatureDep error.
+func NewErrMissingFeatureDep(dep lnwire.FeatureBit) ErrMissingFeatureDep {
+	return ErrMissingFeatureDep{dep: dep}
+}
+
 // Error returns a human-readable description of the missing dep error.
 func (e ErrMissingFeatureDep) Error() string {
 	return fmt.Sprintf("missing feature dependency: %v", e.dep)
@@ -74,7 +79,7 @@ func validateDeps(features featureSet, supported supportedFeatures) error {
 		// vector is invalid.
 		checked, ok := supported[bit]
 		if !ok {
-			return ErrMissingFeatureDep{bit}
+			return NewErrMissingFeatureDep(bit)
 		}
 
 		// Alternatively, if we know that this depdendency is valid, we
diff --git a/routing/pathfind.go b/routing/pathfind.go
index cb0e5c02..90c4e576 100644
--- a/routing/pathfind.go
+++ b/routing/pathfind.go
@@ -10,6 +10,7 @@ import (
 	"github.com/btcsuite/btcd/btcec"
 	"github.com/coreos/bbolt"
 	"github.com/lightningnetwork/lnd/channeldb"
+	"github.com/lightningnetwork/lnd/feature"
 	"github.com/lightningnetwork/lnd/lnwire"
 	"github.com/lightningnetwork/lnd/record"
 	"github.com/lightningnetwork/lnd/routing/route"
@@ -427,6 +428,17 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
 		}
 	}
 
+	// With the destination's feature vector selected, ensure that all
+	// transitive depdencies are set.
+	err = feature.ValidateDeps(features)
+	if err != nil {
+		return nil, err
+	}
+
+	// Now that we know the feature vector is well formed, we'll proceed in
+	// checking that it supports the features we need, given our
+	// restrictions on the final hop.
+
 	// If the caller needs to send custom records, check that our
 	// destination feature vector supports TLV.
 	if len(r.DestCustomRecords) > 0 &&
diff --git a/routing/pathfind_test.go b/routing/pathfind_test.go
index ea366277..3647d9cb 100644
--- a/routing/pathfind_test.go
+++ b/routing/pathfind_test.go
@@ -22,6 +22,7 @@ import (
 	"github.com/btcsuite/btcd/wire"
 	"github.com/btcsuite/btcutil"
 	"github.com/lightningnetwork/lnd/channeldb"
+	"github.com/lightningnetwork/lnd/feature"
 	"github.com/lightningnetwork/lnd/lnwire"
 	"github.com/lightningnetwork/lnd/record"
 	"github.com/lightningnetwork/lnd/routing/route"
@@ -65,6 +66,19 @@ var (
 			lnwire.TLVOnionPayloadOptional,
 		), lnwire.Features,
 	)
+
+	payAddrFeatures = lnwire.NewFeatureVector(
+		lnwire.NewRawFeatureVector(
+			lnwire.PaymentAddrOptional,
+		), lnwire.Features,
+	)
+
+	tlvPayAddrFeatures = lnwire.NewFeatureVector(
+		lnwire.NewRawFeatureVector(
+			lnwire.TLVOnionPayloadOptional,
+			lnwire.PaymentAddrOptional,
+		), lnwire.Features,
+	)
 )
 
 var (
@@ -1446,6 +1460,78 @@ func TestDestTLVGraphFallback(t *testing.T) {
 	assertExpectedPath(t, ctx.testGraphInstance.aliasMap, path, "luoji")
 }
 
+// TestMissingFeatureDep asserts that we fail path finding when the
+// destination's features are broken, in that the feature vector doesn't signal
+// all transitive dependencies.
+func TestMissingFeatureDep(t *testing.T) {
+	t.Parallel()
+
+	testChannels := []*testChannel{
+		asymmetricTestChannel("roasbeef", "conner", 100000,
+			&testChannelPolicy{
+				Expiry:  144,
+				FeeRate: 400,
+				MinHTLC: 1,
+				MaxHTLC: 100000000,
+			},
+			&testChannelPolicy{
+				Expiry:   144,
+				FeeRate:  400,
+				MinHTLC:  1,
+				MaxHTLC:  100000000,
+				Features: payAddrFeatures,
+			}, 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,
+		)
+	}
+
+	// Conner's node in the graph has a broken feature vector, since it
+	// signals payment addresses without signaling tlv onions. Pathfinding
+	// should fail since we validate transitive feature dependencies for the
+	// final node.
+	conner := ctx.testGraphInstance.aliasMap["conner"]
+
+	restrictions := *noRestrictions
+
+	_, err = find(&restrictions, conner)
+	if err != feature.NewErrMissingFeatureDep(
+		lnwire.TLVOnionPayloadOptional,
+	) {
+		t.Fatalf("path shouldn't have been found: %v", err)
+	}
+
+	// Now, set the TLV and payment addresses features to override the
+	// broken features found in the graph. We should succeed in finding a
+	// path to conner.
+	restrictions.DestFeatures = tlvPayAddrFeatures
+
+	path, err := find(&restrictions, conner)
+	if err != nil {
+		t.Fatalf("path should have been found: %v", err)
+	}
+	assertExpectedPath(t, ctx.testGraphInstance.aliasMap, path, "conner")
+}
+
 func TestPathInsufficientCapacity(t *testing.T) {
 	t.Parallel()