multi: Add ability to set custom node announcement feature bits in config

cmd/lncli/peersrpc_active.go

@@ -126,7 +126,7 @@ func updateNodeAnnouncement(ctx *cli.Context) error {
 
 	if ctx.IsSet("feature_bit_add") {
 		change = true
-		for _, feature := range ctx.IntSlice("feature_bit_add") {
+		for _, feature := range ctx.Int64Slice("feature_bit_add") {
 			action := &peersrpc.UpdateFeatureAction{
 				Action:     peersrpc.UpdateAction_ADD,
 				FeatureBit: lnrpc.FeatureBit(feature),
@@ -137,7 +137,7 @@ func updateNodeAnnouncement(ctx *cli.Context) error {
 
 	if ctx.IsSet("feature_bit_remove") {
 		change = true
-		for _, feature := range ctx.IntSlice("feature_bit_remove") {
+		for _, feature := range ctx.Int64Slice("feature_bit_remove") {
 			action := &peersrpc.UpdateFeatureAction{
 				Action:     peersrpc.UpdateAction_REMOVE,
 				FeatureBit: lnrpc.FeatureBit(feature),

docs/release-notes/release-notes-0.16.0.md

@@ -130,6 +130,14 @@ current gossip sync query status.
 * [A bug has been fixed which could cause `lnd` to crash when parsing a
   malformed HTLC intercept message](https://github.com/lightningnetwork/lnd/pull/7392).
 
+* The [UpdateNodeAnnouncement](https://github.com/lightningnetwork/lnd/pull/7168)
+  API can no longer be used to set/unset protocol features that are defined by 
+  LND. A bug in the `updatenodeannouncement` peers cli which did not allow 
+  setting/unsetting of feature bits also has been fixed. 
+
+  Custom node announcement feature bits can also be specified in config using 
+  the `dev` build tag and `--protocol.custom-nodeann-feature`  flag. 
+
 ## Wallet
 
 * [Allows Taproot public keys and tap scripts to be imported as watch-only

lncfg/protocol_experimental_off.go

@@ -3,6 +3,8 @@
 
 package lncfg
 
+import "github.com/lightningnetwork/lnd/lnwire"
+
 // ExperimentalProtocol is a sub-config that houses any experimental protocol
 // features that also require a build-tag to activate.
 type ExperimentalProtocol struct {
@@ -13,3 +15,9 @@ type ExperimentalProtocol struct {
 func (p ExperimentalProtocol) CustomMessageOverrides() []uint16 {
 	return nil
 }
+
+// CustomAnnFeatures returns a set of protocol feature bits to set in the node's
+// announcement.
+func (p ExperimentalProtocol) CustomAnnFeatures() []lnwire.FeatureBit {
+	return nil
+}

lncfg/protocol_experimental_on.go

@@ -3,16 +3,31 @@
 
 package lncfg
 
+import "github.com/lightningnetwork/lnd/lnwire"
+
 // ExperimentalProtocol is a sub-config that houses any experimental protocol
 // features that also require a build-tag to activate.
 //
 //nolint:lll
 type ExperimentalProtocol struct {
 	CustomMessage []uint16 `long:"custom-message" description:"allows the custom message apis to send and report messages with the protocol number provided that fall outside of the custom message number range."`
+
+	CustomFeature []uint64 `long:"custom-nodeann-feature" description:"a protocol feature bit to set in our node's announcement"`
 }
 
 // CustomMessageOverrides returns the set of protocol messages that we override
 // to allow custom handling.
 func (p ExperimentalProtocol) CustomMessageOverrides() []uint16 {
 	return p.CustomMessage
 }
+
+// CustomAnnFeatures returns a set of protocol feature bits to add to the node's
+// announcement.
+func (p ExperimentalProtocol) CustomAnnFeatures() []lnwire.FeatureBit {
+	features := make([]lnwire.FeatureBit, len(p.CustomFeature))
+	for i, f := range p.CustomFeature {
+		features[i] = lnwire.FeatureBit(f)
+	}
+
+	return features
+}

lnrpc/peersrpc/config_active.go

@@ -26,4 +26,8 @@ type Config struct {
 	// UpdateNodeAnnouncement updates our node announcement applying the
 	// given NodeAnnModifiers and broadcasts the new version to the network.
 	UpdateNodeAnnouncement func(...netann.NodeAnnModifier) error
+
+	// ConfigFeatures holds a set of features that are set in lnd's top
+	// level config.
+	ConfigFeatures []lnwire.FeatureBit
 }

lnrpc/peersrpc/peers_server.go

@@ -248,36 +248,25 @@ func (s *Server) updateFeatures(currentfeatures *lnwire.RawFeatureVector,
 	updates []*UpdateFeatureAction) (*lnwire.RawFeatureVector,
 	*lnrpc.Op, error) {
 
-	ops := &lnrpc.Op{Entity: "features"}
-	raw := currentfeatures.Clone()
+	var (
+		ops    = &lnrpc.Op{Entity: "features"}
+		add    []lnwire.FeatureBit
+		remove []lnwire.FeatureBit
+	)
 
 	for _, update := range updates {
 		bit := lnwire.FeatureBit(update.FeatureBit)
 
 		switch update.Action {
 		case UpdateAction_ADD:
-			if raw.IsSet(bit) {
-				return nil, nil, fmt.Errorf(
-					"invalid add action for bit %v, "+
-						"bit is already set",
-					update.FeatureBit,
-				)
-			}
-			raw.Set(bit)
+			add = append(add, bit)
 			ops.Actions = append(
 				ops.Actions,
 				fmt.Sprintf("%s set", lnwire.Features[bit]),
 			)
 
 		case UpdateAction_REMOVE:
-			if !raw.IsSet(bit) {
-				return nil, nil, fmt.Errorf(
-					"invalid remove action for bit %v, "+
-						"bit is already unset",
-					update.FeatureBit,
-				)
-			}
-			raw.Unset(bit)
+			remove = append(remove, bit)
 			ops.Actions = append(
 				ops.Actions,
 				fmt.Sprintf("%s unset", lnwire.Features[bit]),
@@ -292,7 +281,13 @@ func (s *Server) updateFeatures(currentfeatures *lnwire.RawFeatureVector,
 		}
 	}
 
-	// Validate our new SetNodeAnn.
+	raw, err := lnwire.MergeFeatureVector(
+		currentfeatures, add, remove, s.cfg.ConfigFeatures,
+	)
+	if err != nil {
+		return nil, nil, err
+	}
+
 	fv := lnwire.NewFeatureVector(raw, lnwire.Features)
 	if err := feature.ValidateDeps(fv); err != nil {
 		return nil, nil, fmt.Errorf(

lntest/itest/lnd_channel_graph_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/lightningnetwork/lnd/lntemp"
 	"github.com/lightningnetwork/lnd/lntemp/node"
 	"github.com/lightningnetwork/lnd/lntest/wait"
+	"github.com/lightningnetwork/lnd/lnwire"
 	"github.com/stretchr/testify/require"
 )
 
@@ -426,7 +427,7 @@ func testUpdateNodeAnnouncement(ht *lntemp.HarnessTest) {
 
 	var lndArgs []string
 
-	// Add some exta addresses to the default ones.
+	// Add some extra addresses to the default ones.
 	extraAddrs := []string{
 		"192.168.1.1:8333",
 		"[2001:db8:85a3:8d3:1319:8a2e:370:7348]:8337",
@@ -437,6 +438,13 @@ func testUpdateNodeAnnouncement(ht *lntemp.HarnessTest) {
 	for _, addr := range extraAddrs {
 		lndArgs = append(lndArgs, "--externalip="+addr)
 	}
+
+	// Advertise a custom feature bit in our args at startup.
+	customFeature := lnwire.FeatureBit(1000)
+	lndArgs = append(lndArgs, fmt.Sprintf(
+		"--protocol.custom-nodeann-feature=%v", customFeature,
+	))
+
 	dave := ht.NewNode("Dave", lndArgs)
 
 	// assertNodeAnn is a helper closure that checks a given node update
@@ -478,10 +486,46 @@ func testUpdateNodeAnnouncement(ht *lntemp.HarnessTest) {
 		)
 	}
 
-	// This feature bit is used to test that our endpoint sets/unsets
-	// feature bits properly. If the current FeatureBit is set by default
-	// update this one for another one unset by default at random.
-	featureBit := lnrpc.FeatureBit_WUMBO_CHANNELS_REQ
+	// Assert that the custom feature bit set in our config args is set.
+	_, haveCustom := resp.Features[uint32(customFeature)]
+	require.True(ht, haveCustom, "configured feature bit not "+
+		"advertised: %v", resp.Features)
+
+	// Test that we can't modify features bits that are set in our config.
+	nodeAnnReq := &peersrpc.NodeAnnouncementUpdateRequest{
+		FeatureUpdates: []*peersrpc.UpdateFeatureAction{
+			{
+				Action:     peersrpc.UpdateAction_ADD,
+				FeatureBit: lnrpc.FeatureBit(customFeature),
+			},
+		},
+	}
+	dave.RPC.UpdateNodeAnnouncementErr(nodeAnnReq)
+
+	// Test that we can't modify a feature bit that is known to LND.
+	reservedFeatureBit := lnrpc.FeatureBit_WUMBO_CHANNELS_REQ
+	_, known := lnwire.Features[lnwire.FeatureBit(reservedFeatureBit)]
+	require.True(ht, known, "feature bit should be known to lnd")
+
+	nodeAnnReq = &peersrpc.NodeAnnouncementUpdateRequest{
+		FeatureUpdates: []*peersrpc.UpdateFeatureAction{
+			{
+				Action:     peersrpc.UpdateAction_ADD,
+				FeatureBit: reservedFeatureBit,
+			},
+		},
+	}
+
+	// Node announcement should fail because we're not allowed to alter
+	// "known" feature bits.
+	dave.RPC.UpdateNodeAnnouncementErr(nodeAnnReq)
+
+	// We also set an unknown feature bit (which we should be able to
+	// set/unset) and test that we can update it accordingly.
+	featureBit := lnrpc.FeatureBit(999)
+	_, known = lnwire.Features[lnwire.FeatureBit(featureBit)]
+	require.False(ht, known, "feature bit should be unknown to lnd")
+
 	featureIdx := uint32(featureBit)
 	_, ok := resp.Features[featureIdx]
 	require.False(ht, ok, "unexpected feature bit enabled by default")
@@ -553,7 +597,7 @@ func testUpdateNodeAnnouncement(ht *lntemp.HarnessTest) {
 		},
 	}
 
-	nodeAnnReq := &peersrpc.NodeAnnouncementUpdateRequest{
+	nodeAnnReq = &peersrpc.NodeAnnouncementUpdateRequest{
 		Alias:          newAlias,
 		Color:          newColor,
 		AddressUpdates: updateAddressActions,

lnwire/features.go

@@ -3,13 +3,31 @@ package lnwire
 import (
 	"encoding/binary"
 	"errors"
+	"fmt"
 	"io"
 )
 
 var (
 	// ErrFeaturePairExists signals an error in feature vector construction
 	// where the opposing bit in a feature pair has already been set.
 	ErrFeaturePairExists = errors.New("feature pair exists")
+
+	// ErrFeatureStandard is returned when attempts to modify LND's known
+	// set of features are made.
+	ErrFeatureStandard = errors.New("feature is used in standard " +
+		"protocol set")
+
+	// ErrFeatureSet is returned when an attempt to set a feature bit that
+	// is already active is made.
+	ErrFeatureSet = errors.New("feature bit already set")
+
+	// ErrFeatureNotSet is returned when an attempt to un-set a feature bit
+	// this is not active is made.
+	ErrFeatureNotSet = errors.New("feature bit not set")
+
+	// ErrFeatureConfigured is returned when an attempt to un-set a feature
+	// that was set in lnd's config is made.
+	ErrFeatureConfigured = errors.New("feature bit set in config")
 )
 
 // FeatureBit represents a feature that can be enabled in either a local or
@@ -621,3 +639,64 @@ func (fv *FeatureVector) Clone() *FeatureVector {
 	features := fv.RawFeatureVector.Clone()
 	return NewFeatureVector(features, fv.featureNames)
 }
+
+// MergeFeatureVector accepts a list of feature bits to add and remove,
+// validates that they may be modified (are not known to LND, set/unset actions
+// are logical for the current vector) and returns a merged vector. The merge
+// function also accepts a map of feature bits that were set at a config level.
+// These features are considered "hard-set", so will also trigger an error if
+// we attempt to modify them.
+func MergeFeatureVector(current *RawFeatureVector, add, remove []FeatureBit,
+	configured []FeatureBit) (*RawFeatureVector, error) {
+
+	if current == nil {
+		return nil, errors.New("nil feature vector not allowed")
+	}
+
+	configuredBits := make(map[FeatureBit]struct{}, len(configured))
+	for _, bit := range configured {
+		configuredBits[bit] = struct{}{}
+	}
+
+	// Make a copy of the current vector so that we don't mutate it.
+	raw := current.Clone()
+	for _, bit := range add {
+		if _, set := configuredBits[bit]; set {
+			return nil, fmt.Errorf("can't add feature bit %d: %w",
+				bit, ErrFeatureConfigured)
+		}
+
+		if name, known := Features[bit]; known {
+			return nil, fmt.Errorf("can't set feature "+
+				"bit %d (%v): %w", bit, name,
+				ErrFeatureStandard)
+		}
+
+		if raw.IsSet(bit) {
+			return nil, fmt.Errorf("invalid add action for bit "+
+				"%d: %w", bit, ErrFeatureSet)
+		}
+		raw.Set(bit)
+	}
+
+	for _, bit := range remove {
+		if _, set := configuredBits[bit]; set {
+			return nil, fmt.Errorf("can't remove feature bit %d: "+
+				"%w", bit, ErrFeatureConfigured)
+		}
+
+		if name, known := Features[bit]; known {
+			return nil, fmt.Errorf("can't unset feature "+
+				"bit %d (%v): %w", bit, name,
+				ErrFeatureStandard)
+		}
+
+		if !raw.IsSet(bit) {
+			return nil, fmt.Errorf("invalid remove action for "+
+				"bit %d: %w", bit, ErrFeatureNotSet)
+		}
+		raw.Unset(bit)
+	}
+
+	return raw, nil
+}