From 25e89683f815c9be23d91f22f9d0b8599ad72df2 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Wed, 16 Nov 2022 10:33:20 -0500 Subject: [PATCH 1/8] lncli/bugfix: use int64slice for update node announcement features As is, we'll never get any add/remove features because the flag is set as an int 64 slice but accessed as an int slice (which has no value, and the cli doesn't fail if the flag doesn't exist). --- cmd/lncli/peersrpc_active.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/lncli/peersrpc_active.go b/cmd/lncli/peersrpc_active.go index 094dde1c3d..1636bc99a0 100644 --- a/cmd/lncli/peersrpc_active.go +++ b/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), From 3cfd8a6c6ba076b6257f86e735d281cd6e2f4980 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Wed, 16 Nov 2022 12:09:16 -0500 Subject: [PATCH 2/8] multi: restrict feature modification via rpc to unknown features Do no allow nodes to set/unset feature bits via the RPC that are defined by LND. This change prevents users from accidentally disabling features that are in use (since turning them off in our node ann doesn't actually disable their use elsewhere). --- lnrpc/peersrpc/peers_server.go | 6 +++++ lntest/itest/lnd_channel_graph_test.go | 31 +++++++++++++++++++++----- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/lnrpc/peersrpc/peers_server.go b/lnrpc/peersrpc/peers_server.go index 84516f7dca..28c56e5900 100644 --- a/lnrpc/peersrpc/peers_server.go +++ b/lnrpc/peersrpc/peers_server.go @@ -254,6 +254,12 @@ func (s *Server) updateFeatures(currentfeatures *lnwire.RawFeatureVector, for _, update := range updates { bit := lnwire.FeatureBit(update.FeatureBit) + if name, known := lnwire.Features[bit]; known { + return nil, nil, fmt.Errorf("can't modify feature "+ + "bit: %d already used in LND for: %v", bit, + name) + } + switch update.Action { case UpdateAction_ADD: if raw.IsSet(bit) { diff --git a/lntest/itest/lnd_channel_graph_test.go b/lntest/itest/lnd_channel_graph_test.go index e9e7ae6486..f1f310b284 100644 --- a/lntest/itest/lnd_channel_graph_test.go +++ b/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" ) @@ -478,10 +479,30 @@ 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 + // 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 +574,7 @@ func testUpdateNodeAnnouncement(ht *lntemp.HarnessTest) { }, } - nodeAnnReq := &peersrpc.NodeAnnouncementUpdateRequest{ + nodeAnnReq = &peersrpc.NodeAnnouncementUpdateRequest{ Alias: newAlias, Color: newColor, AddressUpdates: updateAddressActions, From e4990e927a2c37c2ff63978e3328baaa27060d6f Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Thu, 17 Nov 2022 12:14:44 -0500 Subject: [PATCH 3/8] multi: add dedicated merge feature function for use outside of rpc Pull out the feature bit merge logic into lnwire in preparation for adding the ability to set feature bits in config. --- lnrpc/peersrpc/peers_server.go | 37 +++++++--------------- lnwire/features.go | 57 ++++++++++++++++++++++++++++++++++ lnwire/features_test.go | 51 ++++++++++++++++++++++++++++++ 3 files changed, 120 insertions(+), 25 deletions(-) diff --git a/lnrpc/peersrpc/peers_server.go b/lnrpc/peersrpc/peers_server.go index 28c56e5900..be4542d711 100644 --- a/lnrpc/peersrpc/peers_server.go +++ b/lnrpc/peersrpc/peers_server.go @@ -248,42 +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) - if name, known := lnwire.Features[bit]; known { - return nil, nil, fmt.Errorf("can't modify feature "+ - "bit: %d already used in LND for: %v", bit, - name) - } - 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]), @@ -298,7 +281,11 @@ func (s *Server) updateFeatures(currentfeatures *lnwire.RawFeatureVector, } } - // Validate our new SetNodeAnn. + raw, err := lnwire.MergeFeatureVector(currentfeatures, add, remove) + 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( diff --git a/lnwire/features.go b/lnwire/features.go index c386fd37be..35909127d5 100644 --- a/lnwire/features.go +++ b/lnwire/features.go @@ -3,6 +3,7 @@ package lnwire import ( "encoding/binary" "errors" + "fmt" "io" ) @@ -10,6 +11,19 @@ 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") ) // FeatureBit represents a feature that can be enabled in either a local or @@ -621,3 +635,46 @@ 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. +func MergeFeatureVector(current *RawFeatureVector, add, remove []FeatureBit) ( + *RawFeatureVector, error) { + + if current == nil { + return nil, errors.New("nil feature vector not allowed") + } + + // Make a copy of the current vector so that we don't mutate it. + raw := current.Clone() + for _, bit := range add { + 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 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 +} diff --git a/lnwire/features_test.go b/lnwire/features_test.go index a945eeb3e9..92ed4efc82 100644 --- a/lnwire/features_test.go +++ b/lnwire/features_test.go @@ -2,6 +2,7 @@ package lnwire import ( "bytes" + "errors" "reflect" "sort" "testing" @@ -396,3 +397,53 @@ func TestIsEmptyFeatureVector(t *testing.T) { fv.Unset(StaticRemoteKeyOptional) require.True(t, fv.IsEmpty()) } + +// TestMergeFeatureVector tests addition and removal of feature bits from +// a vector. +func TestMergeFeatureVector(t *testing.T) { + t.Parallel() + + features := []FeatureBit{ + StaticRemoteKeyRequired, + } + + // Create our initial feature vector, and clone it so that we can assert + // that it is unchanged. + fv := NewRawFeatureVector(features...) + fcv := fv.Clone() + + // Now, try to add and remove features that are defined by lnd - both + // should fail. + knownFeatures := []FeatureBit{ + ExplicitChannelTypeRequired, + } + _, err := MergeFeatureVector(fv, knownFeatures, nil) + require.True(t, errors.Is(err, ErrFeatureStandard), + "add known feature should fail") + + _, err = MergeFeatureVector(fv, nil, knownFeatures) + require.True(t, errors.Is(err, ErrFeatureStandard), + "remove known features should fail") + + // Now, set a custom feature bit. + customFeatures := []FeatureBit{ + 10001, + } + fv1, err := MergeFeatureVector(fv, customFeatures, nil) + require.NoError(t, err, "custom feature add ok") + + require.True(t, fv1.IsSet(customFeatures[0]), "custom feature not set") + require.Equal(t, fv, fcv, "original vector mutated") + + // Attempt to set the custom feature that we just added, this should + // fail because it's already set. + _, err = MergeFeatureVector(fv1, customFeatures, nil) + require.True(t, errors.Is(err, ErrFeatureSet), + "re-add custom should fail") + + // Unset our custom feature and assert that it's actually removed. + fv2, err := MergeFeatureVector(fv1, nil, customFeatures) + require.NoError(t, err, "remove custom feature should succeed") + + require.False(t, fv2.IsSet(customFeatures[0])) +} From 771ee0281df6e57be81861606b48766f8a2b25d5 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Tue, 13 Dec 2022 11:25:44 -0500 Subject: [PATCH 4/8] multi: update merge function to consider configured features When we add the ability to set features in lnd's config, we want to disallow modifications of those features (since they're "hard set" in config). This commit updates the merge function to prevent a separate set of features provided in config from being modified. --- lnrpc/peersrpc/peers_server.go | 4 +++- lnwire/features.go | 28 ++++++++++++++++++++++++--- lnwire/features_test.go | 35 +++++++++++++++++++++++++++++----- 3 files changed, 58 insertions(+), 9 deletions(-) diff --git a/lnrpc/peersrpc/peers_server.go b/lnrpc/peersrpc/peers_server.go index be4542d711..62fcbd573f 100644 --- a/lnrpc/peersrpc/peers_server.go +++ b/lnrpc/peersrpc/peers_server.go @@ -281,7 +281,9 @@ func (s *Server) updateFeatures(currentfeatures *lnwire.RawFeatureVector, } } - raw, err := lnwire.MergeFeatureVector(currentfeatures, add, remove) + raw, err := lnwire.MergeFeatureVector( + currentfeatures, add, remove, nil, + ) if err != nil { return nil, nil, err } diff --git a/lnwire/features.go b/lnwire/features.go index 35909127d5..0413d5d901 100644 --- a/lnwire/features.go +++ b/lnwire/features.go @@ -24,6 +24,10 @@ var ( // 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 @@ -638,17 +642,30 @@ func (fv *FeatureVector) Clone() *FeatureVector { // 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. -func MergeFeatureVector(current *RawFeatureVector, add, remove []FeatureBit) ( - *RawFeatureVector, error) { +// 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, @@ -663,6 +680,11 @@ func MergeFeatureVector(current *RawFeatureVector, add, remove []FeatureBit) ( } 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, diff --git a/lnwire/features_test.go b/lnwire/features_test.go index 92ed4efc82..7b2b7d106c 100644 --- a/lnwire/features_test.go +++ b/lnwire/features_test.go @@ -417,11 +417,12 @@ func TestMergeFeatureVector(t *testing.T) { knownFeatures := []FeatureBit{ ExplicitChannelTypeRequired, } - _, err := MergeFeatureVector(fv, knownFeatures, nil) + + _, err := MergeFeatureVector(fv, knownFeatures, nil, nil) require.True(t, errors.Is(err, ErrFeatureStandard), "add known feature should fail") - _, err = MergeFeatureVector(fv, nil, knownFeatures) + _, err = MergeFeatureVector(fv, nil, knownFeatures, nil) require.True(t, errors.Is(err, ErrFeatureStandard), "remove known features should fail") @@ -429,7 +430,7 @@ func TestMergeFeatureVector(t *testing.T) { customFeatures := []FeatureBit{ 10001, } - fv1, err := MergeFeatureVector(fv, customFeatures, nil) + fv1, err := MergeFeatureVector(fv, customFeatures, nil, nil) require.NoError(t, err, "custom feature add ok") require.True(t, fv1.IsSet(customFeatures[0]), "custom feature not set") @@ -437,13 +438,37 @@ func TestMergeFeatureVector(t *testing.T) { // Attempt to set the custom feature that we just added, this should // fail because it's already set. - _, err = MergeFeatureVector(fv1, customFeatures, nil) + _, err = MergeFeatureVector(fv1, customFeatures, nil, nil) require.True(t, errors.Is(err, ErrFeatureSet), "re-add custom should fail") // Unset our custom feature and assert that it's actually removed. - fv2, err := MergeFeatureVector(fv1, nil, customFeatures) + fv2, err := MergeFeatureVector(fv1, nil, customFeatures, nil) require.NoError(t, err, "remove custom feature should succeed") require.False(t, fv2.IsSet(customFeatures[0])) + + // Test that we can't add or remove features that are included in lnd's + // config. + configFeatures := []FeatureBit{ + 10001, + } + + _, err = MergeFeatureVector(fv1, customFeatures, nil, configFeatures) + require.True(t, errors.Is(err, ErrFeatureConfigured), + "add configured feature should fail") + + _, err = MergeFeatureVector(fv1, nil, customFeatures, configFeatures) + require.True(t, errors.Is(err, ErrFeatureConfigured), + "remove configured feature should fail") + + // For sanity, check that we can set a feature when we do have some + // config features, but there's no overlap. + addFeature := []FeatureBit{ + 10002, + } + fv3, err := MergeFeatureVector(fv1, addFeature, nil, configFeatures) + require.NoError(t, err, "set distinct to config fails") + + require.True(t, fv3.IsSet(addFeature[0]), "feature not set") } From f8991ca9ad440e941e155c756029a1284648c989 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Wed, 16 Nov 2022 11:00:44 -0500 Subject: [PATCH 5/8] server/refactor: use genNodeAnnouncement for first node announcement sig Re-use the signature generation and update of cached node announcement code provided by genNodeAnnouncement when we first set our current node announcement in the server. There is a slight re-ordering in this refactor: Previously: * Get self announcement * Sign announcement * Set selfNode.AuthSigBytes * Set nodeAnn.Signature * Set chanGraph.SetSourceNode * Set s.currentNodeAnn Now: * Get self announcement and set s.currentNodeAnn * Sign announcement and set nodeAnn.Signature (in genNodeAnnouncement) * Set selfNode.AuthSigBytes * Set chanGraph.SetSourceNode --- server.go | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/server.go b/server.go index 8a3d6fedcc..5247a81c59 100644 --- a/server.go +++ b/server.go @@ -826,34 +826,26 @@ func newServer(cfg *Config, listenAddrs []net.Addr, // Based on the disk representation of the node announcement generated // above, we'll generate a node announcement that can go out on the // network so we can properly sign it. - nodeAnn, err := selfNode.NodeAnnouncement(false) + s.currentNodeAnn, err = selfNode.NodeAnnouncement(false) if err != nil { return nil, fmt.Errorf("unable to gen self node ann: %v", err) } - // With the announcement generated, we'll sign it to properly - // authenticate the message on the network. - authSig, err := netann.SignAnnouncement( - s.nodeSigner, nodeKeyDesc.KeyLocator, nodeAnn, - ) + // With the announcement generated, we'll use our getNodeAnnouncement + // helper to sign our current announcement to properly authenticate + // with the network. + nodeAnn, err := s.genNodeAnnouncement(true) if err != nil { return nil, fmt.Errorf("unable to generate signature for "+ "self node announcement: %v", err) } - selfNode.AuthSigBytes = authSig.Serialize() - nodeAnn.Signature, err = lnwire.NewSigFromRawSignature( - selfNode.AuthSigBytes, - ) - if err != nil { - return nil, err - } + selfNode.AuthSigBytes = nodeAnn.Signature.ToSignatureBytes() // Finally, we'll update the representation on disk, and update our // cached in-memory version as well. if err := chanGraph.SetSourceNode(selfNode); err != nil { return nil, fmt.Errorf("can't set self node: %v", err) } - s.currentNodeAnn = nodeAnn // The router will get access to the payment ID sequencer, such that it // can generate unique payment IDs. From a4f07c238aa80ba2a3e6da1483241e589a76f700 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Tue, 31 Jan 2023 14:43:24 -0500 Subject: [PATCH 6/8] multi: allow setting of custom feature bits in config --- lncfg/protocol_experimental_off.go | 8 ++++++++ lncfg/protocol_experimental_on.go | 15 +++++++++++++++ lnrpc/peersrpc/config_active.go | 4 ++++ lnrpc/peersrpc/peers_server.go | 2 +- rpcserver.go | 4 +++- subrpcserver_config.go | 5 +++++ 6 files changed, 36 insertions(+), 2 deletions(-) diff --git a/lncfg/protocol_experimental_off.go b/lncfg/protocol_experimental_off.go index e88b4b52b6..a6dd4156e6 100644 --- a/lncfg/protocol_experimental_off.go +++ b/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 +} diff --git a/lncfg/protocol_experimental_on.go b/lncfg/protocol_experimental_on.go index cf49890ae1..9da8861a34 100644 --- a/lncfg/protocol_experimental_on.go +++ b/lncfg/protocol_experimental_on.go @@ -3,12 +3,16 @@ 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 @@ -16,3 +20,14 @@ type ExperimentalProtocol struct { 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 +} diff --git a/lnrpc/peersrpc/config_active.go b/lnrpc/peersrpc/config_active.go index 860e7627e1..ef48f4b0f1 100644 --- a/lnrpc/peersrpc/config_active.go +++ b/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 } diff --git a/lnrpc/peersrpc/peers_server.go b/lnrpc/peersrpc/peers_server.go index 62fcbd573f..742a7a0a8b 100644 --- a/lnrpc/peersrpc/peers_server.go +++ b/lnrpc/peersrpc/peers_server.go @@ -282,7 +282,7 @@ func (s *Server) updateFeatures(currentfeatures *lnwire.RawFeatureVector, } raw, err := lnwire.MergeFeatureVector( - currentfeatures, add, remove, nil, + currentfeatures, add, remove, s.cfg.ConfigFeatures, ) if err != nil { return nil, nil, err diff --git a/rpcserver.go b/rpcserver.go index 4b1a16fbd2..468b8fa5b0 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -787,7 +787,9 @@ func (r *rpcServer) addDeps(s *server, macService *macaroons.Service, s.sweeper, tower, s.towerClient, s.anchorTowerClient, r.cfg.net.ResolveTCPAddr, genInvoiceFeatures, genAmpInvoiceFeatures, getNodeAnnouncement, - s.updateAndBrodcastSelfNode, parseAddr, rpcsLog, + s.updateAndBrodcastSelfNode, + s.cfg.ProtocolOptions.ExperimentalProtocol.CustomAnnFeatures(), + parseAddr, rpcsLog, s.aliasMgr.GetPeerAlias, ) if err != nil { diff --git a/subrpcserver_config.go b/subrpcserver_config.go index 2723efaeaf..84ef3b1c85 100644 --- a/subrpcserver_config.go +++ b/subrpcserver_config.go @@ -120,6 +120,7 @@ func (s *subRPCServerConfigs) PopulateDependencies(cfg *Config, genAmpInvoiceFeatures func() *lnwire.FeatureVector, getNodeAnnouncement func() (lnwire.NodeAnnouncement, error), updateNodeAnnouncement func(modifiers ...netann.NodeAnnModifier) error, + configFeatures []lnwire.FeatureBit, parseAddr func(addr string) (net.Addr, error), rpcLogger btclog.Logger, getAlias func(lnwire.ChannelID) (lnwire.ShortChannelID, error)) error { @@ -333,6 +334,10 @@ func (s *subRPCServerConfigs) PopulateDependencies(cfg *Config, reflect.ValueOf(updateNodeAnnouncement), ) + subCfgValue.FieldByName("ConfigFeatures").Set( + reflect.ValueOf(configFeatures), + ) + default: return fmt.Errorf("unknown field: %v, %T", fieldName, cfg) From 779d4709f116f24d296085f5946d05432fd9a317 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Mon, 12 Dec 2022 16:15:41 -0500 Subject: [PATCH 7/8] multi: set feature bits from config on startup --- lntest/itest/lnd_channel_graph_test.go | 27 ++++++++++++++++++++-- server.go | 32 +++++++++++++++++++++++++- 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/lntest/itest/lnd_channel_graph_test.go b/lntest/itest/lnd_channel_graph_test.go index f1f310b284..709dc505d4 100644 --- a/lntest/itest/lnd_channel_graph_test.go +++ b/lntest/itest/lnd_channel_graph_test.go @@ -427,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", @@ -438,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 @@ -479,12 +486,28 @@ func testUpdateNodeAnnouncement(ht *lntemp.HarnessTest) { ) } + // 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{ + nodeAnnReq = &peersrpc.NodeAnnouncementUpdateRequest{ FeatureUpdates: []*peersrpc.UpdateFeatureAction{ { Action: peersrpc.UpdateAction_ADD, diff --git a/server.go b/server.go index 5247a81c59..cccee58d8d 100644 --- a/server.go +++ b/server.go @@ -834,13 +834,43 @@ func newServer(cfg *Config, listenAddrs []net.Addr, // With the announcement generated, we'll use our getNodeAnnouncement // helper to sign our current announcement to properly authenticate // with the network. - nodeAnn, err := s.genNodeAnnouncement(true) + var ( + mods []netann.NodeAnnModifier + customFeatures = s.cfg.ProtocolOptions.CustomAnnFeatures() + ) + + raw, err := lnwire.MergeFeatureVector( + s.currentNodeAnn.Features, customFeatures, nil, + // Set the currently configured feature bit set to + // nil because we're adding our config-set features for + // the first time. + nil, + ) + if err != nil { + return nil, fmt.Errorf("unable to set feature bits: "+ + "%w", err) + } + + // Sanity check dependencies for our feature vector, although + // setting of custom features shouldn't affect dependencies. + fv := lnwire.NewFeatureVector(raw, lnwire.Features) + if err := feature.ValidateDeps(fv); err != nil { + return nil, fmt.Errorf("invalid feature vector: %w", + err) + } + + mods = append(mods, netann.NodeAnnSetFeatures(raw)) + + nodeAnn, err := s.genNodeAnnouncement(true, mods...) if err != nil { return nil, fmt.Errorf("unable to generate signature for "+ "self node announcement: %v", err) } selfNode.AuthSigBytes = nodeAnn.Signature.ToSignatureBytes() + // Update the feature bits for the SetNodeAnn in case they changed. + s.featureMgr.SetRaw(feature.SetNodeAnn, nodeAnn.Features) + // Finally, we'll update the representation on disk, and update our // cached in-memory version as well. if err := chanGraph.SetSourceNode(selfNode); err != nil { From 0133ffc506cf5bb17145f5b454514d851255db58 Mon Sep 17 00:00:00 2001 From: Carla Kirk-Cohen Date: Thu, 17 Nov 2022 15:40:00 -0500 Subject: [PATCH 8/8] release-notes: add UpdateNodeAnnouncement changes --- docs/release-notes/release-notes-0.16.0.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/release-notes/release-notes-0.16.0.md b/docs/release-notes/release-notes-0.16.0.md index 7e2c1d5e4d..d49f09be41 100644 --- a/docs/release-notes/release-notes-0.16.0.md +++ b/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