multi: update node announcement features in feature manager first

Move merge of our features into the feature manager, rather than
updating our node announcement then retrospectively setting the
feature manger's set to the new vector. This allows us to use the
feature vector as our single source of truth for announcements.

itest/lnd_channel_graph_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/lightningnetwork/lnd/lntest"
 	"github.com/lightningnetwork/lnd/lntest/node"
 	"github.com/lightningnetwork/lnd/lntest/wait"
+	"github.com/lightningnetwork/lnd/lnwire"
 	"github.com/stretchr/testify/require"
 )
 
@@ -491,10 +492,29 @@ func testUpdateNodeAnnouncement(ht *lntest.HarnessTest) {
 		)
 	}
 
-	// This feature bit is used to test that our endpoint sets/unsets
+	// Test that we can't modify a feature bit that is known to LND.
-	// feature bits properly. If the current FeatureBit is set by default
+	reservedFeatureBit := lnrpc.FeatureBit_WUMBO_CHANNELS_REQ
-	// update this one for another one unset by default at random.
+	_, known := lnwire.Features[lnwire.FeatureBit(reservedFeatureBit)]
-	featureBit := lnrpc.FeatureBit_WUMBO_CHANNELS_REQ
+	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")
@@ -566,7 +586,7 @@ func testUpdateNodeAnnouncement(ht *lntest.HarnessTest) {
 		},
 	}
 
-	nodeAnnReq := &peersrpc.NodeAnnouncementUpdateRequest{
+	nodeAnnReq = &peersrpc.NodeAnnouncementUpdateRequest{
 		Alias:          newAlias,
 		Color:          newColor,
 		AddressUpdates: updateAddressActions,

lnrpc/peersrpc/config_active.go

@@ -23,7 +23,10 @@ type Config struct {
 	// ParseAddr parses an address from its string format to a net.Addr.
 	ParseAddr func(addr string) (net.Addr, error)
 
-	// UpdateNodeAnnouncement updates our node announcement applying the
+	// UpdateNodeAnnouncement updates and broadcasts our node announcement,
-	// given NodeAnnModifiers and broadcasts the new version to the network.
+	// setting the feature vector provided and applying the
-	UpdateNodeAnnouncement func(...netann.NodeAnnModifier) error
+	// NodeAnnModifiers. If no feature updates are required, a nil feature
+	// vector should be provided.
+	UpdateNodeAnnouncement func(features *lnwire.RawFeatureVector,
+		mods ...netann.NodeAnnModifier) error
 }

lnrpc/peersrpc/peers_server.go

@@ -315,20 +315,21 @@ func (s *Server) UpdateNodeAnnouncement(_ context.Context,
 
 	currentNodeAnn := s.cfg.GetNodeAnnouncement()
 
-	if len(req.FeatureUpdates) > 0 {
+	nodeAnnFeatures := currentNodeAnn.Features
-		features, ops, err := s.updateFeatures(
+	featureUpdates := len(req.FeatureUpdates) > 0
-			currentNodeAnn.Features,
+	if featureUpdates {
-			req.FeatureUpdates,
+		var (
+			ops *lnrpc.Op
+			err error
+		)
+		nodeAnnFeatures, ops, err = s.updateFeatures(
+			nodeAnnFeatures, req.FeatureUpdates,
 		)
 		if err != nil {
 			return nil, fmt.Errorf("error trying to update node "+
 				"features: %w", err)
 		}
 		resp.Ops = append(resp.Ops, ops)
-		nodeModifiers = append(
-			nodeModifiers,
-			netann.NodeAnnSetFeatures(features),
-		)
 	}
 
 	if req.Color != "" {
@@ -386,12 +387,14 @@ func (s *Server) UpdateNodeAnnouncement(_ context.Context,
 		)
 	}
 
-	if len(nodeModifiers) == 0 {
+	if len(nodeModifiers) == 0 && !featureUpdates {
-		return nil, fmt.Errorf("unable detect any new values to " +
+		return nil, fmt.Errorf("unable to detect any new values to " +
 			"update the node announcement")
 	}
 
-	if err := s.cfg.UpdateNodeAnnouncement(nodeModifiers...); err != nil {
+	if err := s.cfg.UpdateNodeAnnouncement(
+		nodeAnnFeatures, nodeModifiers...,
+	); err != nil {
 		return nil, err
 	}
 

netann/host_ann.go

@@ -178,7 +178,7 @@ func IPAnnouncer(annUpdater NodeAnnUpdater) func([]net.Addr,
 	map[string]struct{}) error {
 
 	return func(newAddrs []net.Addr, oldAddrs map[string]struct{}) error {
-		_, err := annUpdater(func(
+		_, err := annUpdater(nil, func(
 			currentNodeAnn *lnwire.NodeAnnouncement) {
 			// To ensure we don't duplicate any addresses, we'll
 			// filter out the same of addresses we should no longer

server.go

@@ -1004,7 +1004,7 @@ func newServer(cfg *Config, listenAddrs []net.Addr,
 		UpdateSelfAnnouncement: func() (lnwire.NodeAnnouncement,
 			error) {
 
-			return s.genNodeAnnouncement()
+			return s.genNodeAnnouncement(nil)
 		},
 		ProofMatureDelta:        0,
 		TrickleDelay:            time.Millisecond * time.Duration(cfg.TrickleDelay),
@@ -1290,7 +1290,7 @@ func newServer(cfg *Config, listenAddrs []net.Addr,
 		CurrentNodeAnnouncement: func() (lnwire.NodeAnnouncement,
 			error) {
 
-			return s.genNodeAnnouncement()
+			return s.genNodeAnnouncement(nil)
 		},
 		SendAnnouncement:     s.authGossiper.ProcessLocalAnnouncement,
 		NotifyWhenOnline:     s.NotifyWhenOnline,
@@ -1631,8 +1631,15 @@ func newServer(cfg *Config, listenAddrs []net.Addr,
 					cfg.net.ResolveTCPAddr,
 				)
 			},
-			AdvertisedIPs:  advertisedIPs,
+			AdvertisedIPs: advertisedIPs,
-			AnnounceNewIPs: netann.IPAnnouncer(s.genNodeAnnouncement),
+			AnnounceNewIPs: netann.IPAnnouncer(
+				func(modifier ...netann.NodeAnnModifier) (
+					lnwire.NodeAnnouncement, error) {
+
+					return s.genNodeAnnouncement(
+						nil, modifier...,
+					)
+				}),
 		})
 	}
 
@@ -2505,7 +2512,7 @@ out:
 			// announcement with the updated addresses and broadcast
 			// it to our peers.
 			newNodeAnn, err := s.genNodeAnnouncement(
-				netann.NodeAnnSetAddrs(newAddrs),
+				nil, netann.NodeAnnSetAddrs(newAddrs),
 			)
 			if err != nil {
 				srvrLog.Debugf("Unable to generate new node "+
@@ -2894,7 +2901,7 @@ func (s *server) createNewHiddenService() error {
 	// Now that the onion service has been created, we'll add the onion
 	// address it can be reached at to our list of advertised addresses.
 	newNodeAnn, err := s.genNodeAnnouncement(
-		func(currentAnn *lnwire.NodeAnnouncement) {
+		nil, func(currentAnn *lnwire.NodeAnnouncement) {
 			currentAnn.Addresses = append(currentAnn.Addresses, addr)
 		},
 	)
@@ -2955,12 +2962,31 @@ func (s *server) getNodeAnnouncement() lnwire.NodeAnnouncement {
 // genNodeAnnouncement generates and returns the current fully signed node
 // announcement. The time stamp of the announcement will be updated in order
 // to ensure it propagates through the network.
-func (s *server) genNodeAnnouncement(modifiers ...netann.NodeAnnModifier) (
+func (s *server) genNodeAnnouncement(features *lnwire.RawFeatureVector,
-	lnwire.NodeAnnouncement, error) {
+	modifiers ...netann.NodeAnnModifier) (lnwire.NodeAnnouncement, error) {
 
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
+	// First, try to update our feature manager with the updated set of
+	// features.
+	if features != nil {
+		proposedFeatures := map[feature.Set]*lnwire.RawFeatureVector{
+			feature.SetNodeAnn: features,
+		}
+		err := s.featureMgr.UpdateFeatureSets(proposedFeatures)
+		if err != nil {
+			return lnwire.NodeAnnouncement{}, err
+		}
+
+		// If we could successfully update our feature manager, add
+		// an update modifier to include these new features to our
+		// set.
+		modifiers = append(
+			modifiers, netann.NodeAnnSetFeatures(features),
+		)
+	}
+
 	// Always update the timestamp when refreshing to ensure the update
 	// propagates.
 	modifiers = append(modifiers, netann.NodeAnnSetTimestamp)
@@ -2985,10 +3011,10 @@ func (s *server) genNodeAnnouncement(modifiers ...netann.NodeAnnModifier) (
 // applying the giving modifiers and updating the time stamp
 // to ensure it propagates through the network. Then it brodcasts
 // it to the network.
-func (s *server) updateAndBrodcastSelfNode(
+func (s *server) updateAndBrodcastSelfNode(features *lnwire.RawFeatureVector,
 	modifiers ...netann.NodeAnnModifier) error {
 
-	newNodeAnn, err := s.genNodeAnnouncement(modifiers...)
+	newNodeAnn, err := s.genNodeAnnouncement(features, modifiers...)
 	if err != nil {
 		return fmt.Errorf("unable to generate new node "+
 			"announcement: %v", err)
@@ -3006,9 +3032,7 @@ func (s *server) updateAndBrodcastSelfNode(
 	selfNode.LastUpdate = time.Unix(int64(newNodeAnn.Timestamp), 0)
 	selfNode.Addresses = newNodeAnn.Addresses
 	selfNode.Alias = newNodeAnn.Alias.String()
-	selfNode.Features = lnwire.NewFeatureVector(
+	selfNode.Features = s.featureMgr.Get(feature.SetNodeAnn)
-		newNodeAnn.Features, lnwire.Features,
-	)
 	selfNode.Color = newNodeAnn.RGBColor
 	selfNode.AuthSigBytes = newNodeAnn.Signature.ToSignatureBytes()
 
@@ -3018,11 +3042,6 @@ func (s *server) updateAndBrodcastSelfNode(
 		return fmt.Errorf("can't set self node: %v", err)
 	}
 
-	// Update the feature bits for the SetNodeAnn in case they changed.
-	s.featureMgr.SetRaw(
-		feature.SetNodeAnn, selfNode.Features.RawFeatureVector,
-	)
-
 	// Finally, propagate it to the nodes in the network.
 	err = s.BroadcastMessage(nil, &newNodeAnn)
 	if err != nil {
@@ -3828,7 +3847,11 @@ func (s *server) peerConnected(conn net.Conn, connReq *connmgr.ConnReq,
 		TowerClient:             s.towerClient,
 		AnchorTowerClient:       s.anchorTowerClient,
 		DisconnectPeer:          s.DisconnectPeer,
-		GenNodeAnnouncement:     s.genNodeAnnouncement,
+		GenNodeAnnouncement: func(...netann.NodeAnnModifier) (
+			lnwire.NodeAnnouncement, error) {
+
+			return s.genNodeAnnouncement(nil)
+		},
 
 		PongBuf: s.pongBuf,
 

subrpcserver_config.go

@@ -119,7 +119,8 @@ func (s *subRPCServerConfigs) PopulateDependencies(cfg *Config,
 	genInvoiceFeatures func() *lnwire.FeatureVector,
 	genAmpInvoiceFeatures func() *lnwire.FeatureVector,
 	getNodeAnnouncement func() lnwire.NodeAnnouncement,
-	updateNodeAnnouncement func(modifiers ...netann.NodeAnnModifier) error,
+	updateNodeAnnouncement func(features *lnwire.RawFeatureVector,
+		modifiers ...netann.NodeAnnModifier) error,
 	parseAddr func(addr string) (net.Addr, error),
 	rpcLogger btclog.Logger,
 	getAlias func(lnwire.ChannelID) (lnwire.ShortChannelID, error)) error {