From e41c65785f3064a1d2853fd820dc9c56d79aecdd Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Mon, 3 Apr 2023 11:27:14 +0200 Subject: [PATCH] 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 | 30 +++++++++++++--- lnrpc/peersrpc/config_active.go | 9 +++-- lnrpc/peersrpc/peers_server.go | 25 ++++++++------ netann/host_ann.go | 2 +- server.go | 61 +++++++++++++++++++++++---------- subrpcserver_config.go | 3 +- 6 files changed, 90 insertions(+), 40 deletions(-) diff --git a/itest/lnd_channel_graph_test.go b/itest/lnd_channel_graph_test.go index ed4dfa25a..73a9266b7 100644 --- a/itest/lnd_channel_graph_test.go +++ b/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 - // 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 + // 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") @@ -566,7 +586,7 @@ func testUpdateNodeAnnouncement(ht *lntest.HarnessTest) { }, } - nodeAnnReq := &peersrpc.NodeAnnouncementUpdateRequest{ + nodeAnnReq = &peersrpc.NodeAnnouncementUpdateRequest{ Alias: newAlias, Color: newColor, AddressUpdates: updateAddressActions, diff --git a/lnrpc/peersrpc/config_active.go b/lnrpc/peersrpc/config_active.go index 34da09699..4a2f028e4 100644 --- a/lnrpc/peersrpc/config_active.go +++ b/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 - // given NodeAnnModifiers and broadcasts the new version to the network. - UpdateNodeAnnouncement func(...netann.NodeAnnModifier) error + // UpdateNodeAnnouncement updates and broadcasts our node announcement, + // setting the feature vector provided and applying the + // NodeAnnModifiers. If no feature updates are required, a nil feature + // vector should be provided. + UpdateNodeAnnouncement func(features *lnwire.RawFeatureVector, + mods ...netann.NodeAnnModifier) error } diff --git a/lnrpc/peersrpc/peers_server.go b/lnrpc/peersrpc/peers_server.go index 5710f5fca..6981f7949 100644 --- a/lnrpc/peersrpc/peers_server.go +++ b/lnrpc/peersrpc/peers_server.go @@ -315,20 +315,21 @@ func (s *Server) UpdateNodeAnnouncement(_ context.Context, currentNodeAnn := s.cfg.GetNodeAnnouncement() - if len(req.FeatureUpdates) > 0 { - features, ops, err := s.updateFeatures( - currentNodeAnn.Features, - req.FeatureUpdates, + nodeAnnFeatures := currentNodeAnn.Features + featureUpdates := len(req.FeatureUpdates) > 0 + if 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 { - return nil, fmt.Errorf("unable detect any new values to " + + if len(nodeModifiers) == 0 && !featureUpdates { + 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 } diff --git a/netann/host_ann.go b/netann/host_ann.go index 767c3089c..fdb48e839 100644 --- a/netann/host_ann.go +++ b/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 diff --git a/server.go b/server.go index 9957abbda..d70324d81 100644 --- a/server.go +++ b/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, - AnnounceNewIPs: netann.IPAnnouncer(s.genNodeAnnouncement), + AdvertisedIPs: advertisedIPs, + 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) ( - lnwire.NodeAnnouncement, error) { +func (s *server) genNodeAnnouncement(features *lnwire.RawFeatureVector, + 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( - newNodeAnn.Features, lnwire.Features, - ) + selfNode.Features = s.featureMgr.Get(feature.SetNodeAnn) 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, diff --git a/subrpcserver_config.go b/subrpcserver_config.go index 8de66d1ad..3e7e77bd9 100644 --- a/subrpcserver_config.go +++ b/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 {