feature, routing: treat 266 in a node announcement as unknown

docs/independent-secrets.md

@@ -28,6 +28,10 @@ generates its own: they still come from its shachain.
   bit 267 is set in `init` and `node_announcement`. A new channel uses the
   add-on when both peers signal it and agree on it in `channel_type`. With a
   peer that doesn't signal it, the channel opens the normal way.
+- **The even bit, 266,** is only valid in `channel_type`. A peer that sets it
+  in `init` is refused, and a node that sets it in `node_announcement` is
+  treated as requiring a feature lnd doesn't know: it isn't paid from its
+  announcement or routed through.
 - **Storage.** Each secret is stored with the revocation log entry of the
   commitment it revokes, written in the same database transaction. That's 38
   bytes per revoked state (a 32-byte secret in a TLV record), next to data the

feature/independent_secrets_test.go

@@ -1,6 +1,7 @@
 package feature
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/lightningnetwork/lnd/lnwire"
@@ -81,3 +82,31 @@ func TestCustomFeaturesRefuseKnownBits(t *testing.T) {
 		require.NoError(t, err, "set %v", set)
 	}
 }
+
+// TestValidateRoutableRefusesIndependentSecretsEvenBit checks that a node
+// announcing the even bit of option_independent_secrets, which is only valid in
+// channel_type, is refused for routing as if the bit were unknown, while one
+// announcing the optional bit is not.
+func TestValidateRoutableRefusesIndependentSecretsEvenBit(t *testing.T) {
+	t.Parallel()
+
+	announced := func(bit lnwire.FeatureBit) *lnwire.FeatureVector {
+		return lnwire.NewFeatureVector(lnwire.NewRawFeatureVector(
+			lnwire.ExplicitChannelTypeOptional, bit,
+		), lnwire.Features)
+	}
+
+	even := announced(lnwire.IndependentSecretsRequired)
+	require.NoError(t, ValidateRequired(even), "lnd knows the bit")
+	err := ValidateRoutable(even)
+	require.Error(t, err)
+	require.Contains(t, err.Error(),
+		fmt.Sprintf("%d", lnwire.IndependentSecretsRequired))
+
+	require.NoError(t, ValidateRoutable(
+		announced(lnwire.IndependentSecretsOptional),
+	))
+	require.Error(t, ValidateRoutable(lnwire.NewFeatureVector(
+		lnwire.NewRawFeatureVector(100), lnwire.Features,
+	)), "an unknown required bit is still refused")
+}

feature/required.go

@@ -35,3 +35,23 @@ func ValidateRequired(fv *lnwire.FeatureVector) error {
 	}
 	return nil
 }
+
+// ValidateRoutable returns an error if a node with this feature vector, taken
+// from its node_announcement or from an invoice, can't be paid or routed
+// through: it requires a feature bit we don't know, or it sets the even bit of
+// option_independent_secrets. That bit is only valid in channel_type, and BOLT
+// 3 has a node that receives it in node_announcement treat it as an unknown
+// even bit, as a node that doesn't know the bit already does.
+func ValidateRoutable(fv *lnwire.FeatureVector) error {
+	if err := ValidateRequired(fv); err != nil {
+		return err
+	}
+
+	if fv.IsSet(lnwire.IndependentSecretsRequired) {
+		return NewErrUnknownRequired([]lnwire.FeatureBit{
+			lnwire.IndependentSecretsRequired,
+		})
+	}
+
+	return nil
+}

routing/pathfind.go

@@ -632,7 +632,7 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
 
 	// Ensure that the destination's features don't include unknown
 	// required features.
-	err := feature.ValidateRequired(features)
+	err := feature.ValidateRoutable(features)
 	if err != nil {
 		log.Warnf("Pathfinding destination node features: %v", err)
 		return nil, 0, errUnknownRequiredFeature
@@ -1031,7 +1031,7 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
 
 		// Don't route through nodes that contain unknown required
 		// features and mark as nil in the cache.
-		err = feature.ValidateRequired(fromFeatures)
+		err = feature.ValidateRoutable(fromFeatures)
 		if err != nil {
 			featureCache[node] = nil
 			return nil, nil

routing/pathfind_test.go

@@ -89,6 +89,16 @@ var (
 	unknownRequiredFeatures = lnwire.NewFeatureVector(
 		lnwire.NewRawFeatureVector(100), lnwire.Features,
 	)
+
+	// independentSecretsEvenBitFeatures sets the even bit of
+	// option_independent_secrets, with the bit it depends on, as a node
+	// that announces it would.
+	independentSecretsEvenBitFeatures = lnwire.NewFeatureVector(
+		lnwire.NewRawFeatureVector(
+			lnwire.ExplicitChannelTypeOptional,
+			lnwire.IndependentSecretsRequired,
+		), lnwire.Features,
+	)
 )
 
 var (
@@ -850,6 +860,9 @@ func TestPathFinding(t *testing.T) {
 	}, {
 		name: "unknown required features",
 		fn:   runUnknownRequiredFeatures,
+	}, {
+		name: "independent secrets even bit in node announcement",
+		fn:   runIndependentSecretsEvenBit,
 	}, {
 		name: "destination payment address",
 		fn:   runDestPaymentAddr,
@@ -2011,6 +2024,23 @@ func runMissingFeatureDep(t *testing.T, useCache bool) {
 // destination requires an unknown required feature, and that we skip
 // intermediaries that signal unknown required features.
 func runUnknownRequiredFeatures(t *testing.T, useCache bool) {
+	runRefusedNodeFeatures(t, useCache, unknownRequiredFeatures)
+}
+
+// runIndependentSecretsEvenBit asserts that a node announcing the even bit of
+// option_independent_secrets, which is only valid in channel_type, is treated
+// like one with an unknown required feature: it isn't paid from its
+// announcement, and it isn't routed through.
+func runIndependentSecretsEvenBit(t *testing.T, useCache bool) {
+	runRefusedNodeFeatures(t, useCache, independentSecretsEvenBitFeatures)
+}
+
+// runRefusedNodeFeatures asserts that we fail path finding when the
+// destination's announced features are refused, and that we skip
+// intermediaries whose announced features are refused.
+func runRefusedNodeFeatures(t *testing.T, useCache bool,
+	refused *lnwire.FeatureVector) {
+
 	testChannels := []*testChannel{
 		asymmetricTestChannel("roasbeef", "conner", 100000,
 			&testChannelPolicy{
@@ -2024,7 +2054,7 @@ func runUnknownRequiredFeatures(t *testing.T, useCache bool) {
 				FeeRate:  400,
 				MinHTLC:  1,
 				MaxHTLC:  100000000,
-				Features: unknownRequiredFeatures,
+				Features: refused,
 			}, 0,
 		),
 		asymmetricTestChannel("conner", "joost", 100000,
@@ -2033,7 +2063,7 @@ func runUnknownRequiredFeatures(t *testing.T, useCache bool) {
 				FeeRate:  400,
 				MinHTLC:  1,
 				MaxHTLC:  100000000,
-				Features: unknownRequiredFeatures,
+				Features: refused,
 			},
 			&testChannelPolicy{
 				Expiry:  144,
@@ -2049,21 +2079,20 @@ func runUnknownRequiredFeatures(t *testing.T, useCache bool) {
 	conner := ctx.keyFromAlias("conner")
 	joost := ctx.keyFromAlias("joost")
 
-	// Conner's node in the graph has an unknown required feature (100).
-	// Pathfinding should fail since we check the destination's features for
-	// unknown required features before beginning pathfinding.
+	// Conner's node in the graph has a refused feature vector.
+	// Pathfinding should fail since we check the destination's features
+	// before beginning pathfinding.
 	_, err := ctx.findPath(conner, 100)
 	if !reflect.DeepEqual(err, errUnknownRequiredFeature) {
 		t.Fatalf("path shouldn't have been found: %v", err)
 	}
 
 	// Now, try to find a route to joost through conner. The destination
-	// features are valid, but conner's feature vector in the graph still
-	// requires feature 100. We expect errNoPathFound and not the error
-	// above since intermediate hops are simply skipped if they have invalid
+	// features are valid, but conner's feature vector in the graph is
+	// still refused. We expect errNoPathFound and not the error above
+	// since intermediate hops are simply skipped if they have invalid
 	// feature vectors, leaving no possible route to joost. This asserts
-	// that we don't try to route _through_ nodes with unknown required
-	// features.
+	// that we don't try to route _through_ such nodes.
 	_, err = ctx.findPath(joost, 100)
 	if err != errNoPathFound {
 		t.Fatalf("path shouldn't have been found: %v", err)